[GitHub] [incubator-echarts] plainheart commented on a change in pull request #13039: fix(time): tooltip formatter for time axis

[GitBox <gi...@apache.org>]

plainheart commented on a change in pull request #13039:
URL: https://github.com/apache/incubator-echarts/pull/13039#discussion_r462147069



##########
File path: src/util/time.ts
##########
@@ -224,3 +224,21 @@ export function getUnitValue (
             return date['get' + utc + 'Milliseconds']();
     }
 }
+
+/**
+ * Get Date instance from a date string
+ *
+ * @param {string} date should be in date string or second string
+ *                      e.g., 2020-01-01 or 2020-01-01 02:00:20
+ */
+export function getDateFromStr(date: string | Date): Date {
+    if (!date || date instanceof Date) {
+        return date as Date;
+    }
+    if (date.indexOf(':') < 0) {
+        return new Date(date + ' 00:00:00');
+    }
+    else {
+        return new Date(date);
+    }
+}

Review comment:
       A potential bug may exist in this function `getDateFromStr`, that is, it doesn't handle the case like `2020-07-29 12:25:10` which occurs in the Safari browser. Safari can't parse a date string including `'-'` and will return `Invalid Date`. Therefore, to be compatible with this case, what I want to suggest is using `numberUtil.parseDate` or replacing `'-'` with `'/'` before calling `new Date`.
   If it's my fault to understand the intention of this function, could you let me know? Thank you! 

##########
File path: src/util/time.ts
##########
@@ -224,3 +224,21 @@ export function getUnitValue (
             return date['get' + utc + 'Milliseconds']();
     }
 }
+
+/**
+ * Get Date instance from a date string
+ *
+ * @param {string} date should be in date string or second string
+ *                      e.g., 2020-01-01 or 2020-01-01 02:00:20
+ */
+export function getDateFromStr(date: string | Date): Date {
+    if (!date || date instanceof Date) {
+        return date as Date;
+    }
+    if (date.indexOf(':') < 0) {
+        return new Date(date + ' 00:00:00');
+    }
+    else {
+        return new Date(date);
+    }
+}

Review comment:
       Here is the same issue. https://github.com/apache/incubator-echarts/blob/2a909d9cfdf43714b1cfe90bd46cf48e6945ac79/src/util/time.ts#L147




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org