-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug: Date display is inconsistent #715
Comments
@asaadmahmood, not a high priority, but assigning this to you for UX review. the main thing is to consider what our default date format should be (I propose MMM DD, YYYY because it's unambiguous), and what our supported date formats should be. Other Mattermost components (including Playbooks and Admin Console, etc.) have dates too, and we should (P3) ideally drive some general consensus. |
@chenilim We have a similar date format for Playbooks and the Messages (in the center channel etc), which is Month Day Year (if its not the current year). I've updated the Expected Behaviour to include that. |
Thanks @asaadmahmood! I've updated the recommended change to use the "long month" display, MMMM or July vs. Jul to be consistent with the rest of Mattermost. Setting this as up for grabs. |
@chenilim @jespino I am going to work on this issue. Do you think we can/should store UserSettings in the redux store? |
It turns out that Card View uses moment.js formatting for the date and it has support for strings like But Card Preview on the board uses @chenilim do we want to switch @asaadmahmood the recommended change tells to extend the date formats. Do I understand correctly that we want all 4 variants in the menu like this: |
Yup, thanks @asaadmahmood! After discussion, we decided to align with the current Mattermost behavior for now, i.e.:
Remove the "Set date format" user setting for now. The above should be appropriate internationally. On implementation, let's try to use react-intl instead of momentjs if possible. Please LMK if anyone sees any issues with this. Thanks. |
@chenilim skipping year is really easy with Another big question is the input by user via directly typing something into the input element like here (its possible not to interact with calendar popup and just type date): This doesn't seem an easy task at all because formats are very different for different locales and parsing is not available inside |
Hi @kamre, the internationalized dates above look good to me. The key thing is the dates are unambiguous, which they appear to be. (Updated after taking a closer look): As a workaround for entering the format directly, I propose switching to the MM/DD/YYYY or DD/MM/YYYY format when editing, which is the current behavior. So, let's keep the user setting for now, which will only affect the edit-time display. The broader issue here is that it's easy for the user to enter far-off dates quickly, and direct-entry seems to be the only way with this control. Thoughts @asaadmahmood? |
@chenilim @asaadmahmood I have implemented the proposed approach with different date formats for displaying and editing. Please look at #748, there are some sceenshots. |
* Changes in function `Utils.displayDate`: - uses 'MMMM DD' for dates with current year - uses 'MMMM DD, YYYY' for other years - corresponding unit tests added * Changes for input inside `EditableDayPicker`: - uses `Utils.displayDate` when day picker is not visible - uses preferred date format while day picker is visible - uses preferred date format for input placeholder Co-authored-by: Chen-I Lim <[email protected]> Co-authored-by: Harshil Sharma <[email protected]> Co-authored-by: Scott Bishel <[email protected]>
Steps to reproduce the behavior:
Expected behavior:
The date display should be consistent between the Card view and Card preview, which should be as follows:
If its the current year: February 25
If its not the current year: February 25, 2020
Screenshots:
Date in card view:
Date in card preview:
Platform:
Additional context:
Also, the date display on the Card view follows the Date format user setting, but the Card preview does not.
Recommended change:
The text was updated successfully, but these errors were encountered: