Skip to content
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

Add option to show full date and select date format #1174

Merged
merged 5 commits into from
Mar 10, 2024

Conversation

hjiangsu
Copy link
Member

@hjiangsu hjiangsu commented Mar 7, 2024

Pull Request Description

This PR adds the ability to show full dates on posts, and select a date format from a pre-selected list. In addition, I added a way to add subtitles to a given PickerItem

Issue Being Fixed

Issue Number: #1114

Screenshots / Recordings

1adfd121-bbfe-4b50-b732-fc2f2fdb1e5e.mp4

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

@K4LCIFER
Copy link
Contributor

K4LCIFER commented Mar 7, 2024

Looks awesome! I would maybe make the date more of a grey color so that it isn't as jarring. Also would it be possible for the user to specify their own date format? It seems unnecessary to try and specify every possible date combination that a user may want. I would use one as a sane default (probably something of the form "Mar 7, 2024 12:23") and then allow the user to specify alternatives if they wish.

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super cool!

The only thing I could possibly suggest is to have a "system" setting where the date format follows the locale of the device. This is pretty common in date formatting, and it looks like it is possible in Dart by constructing a DateFormat() without any parameters.


EDIT: I take it back. Constructing a DateFormat() with no parameters always falls back to a hard-coded format, yMMMMd plus jms.

https://github.com/dart-lang/i18n/blob/c6c6c25030c88ca4db7cacc6ad312c8f3d1ba357/pkgs/intl/lib/src/intl/date_format.dart#L724-L727

I will see if there's another way to construct a local format.


EDIT2: Ok, sorry for all the edits! We can use the existing yMMMMd and jms formats while passing the current locale.

final DateTime now = DateTime.now();
  
print(Intl.systemLocale); // Prints: en_US
DateFormat dateFormat = DateFormat.yMMMMd(Intl.systemLocale);
DateFormat timeFormat = DateFormat.jms(Intl.systemLocale);
print('${dateFormat.format(now)} ${timeFormat.format(now)}'); // Prints: March 7, 2024 3:59:22 PM

To test that it really does respect the locale...

final DateTime now = DateTime.now();
  
initializeDateFormatting('en_GB');
DateFormat dateFormat = DateFormat.yMMMMd('en_GB');
DateFormat timeFormat = DateFormat.jms('en_GB');
print('${dateFormat.format(now)} ${timeFormat.format(now)}'); // Prints: 7 March 2024 16:04:04

I completely agree with @K4LCIFER on having a "sane default", but unfortunately that means different things to different people around the world. 😊

@hjiangsu
Copy link
Member Author

hjiangsu commented Mar 8, 2024

I would maybe make the date more of a grey color so that it isn't as jarring

I can maybe adjust the date colour if using custom dates, otherwise it'll default to what it is now!

would it be possible for the user to specify their own date format?

This was my plan for a future enhancement! The reason why I opted to show a few pre-selected options for now is to reduce complexity of the feature. To allow for user-specific date formats, there would be some additional requirement to validate the user input, or implement it in a way that's intuitive to use (e.g., maybe something similar to the post card metadata customization with chips for different sections)

@hjiangsu
Copy link
Member Author

hjiangsu commented Mar 8, 2024

The only thing I could possibly suggest is to have a "system" setting where the date format follows the locale of the device.

I like this suggestion! I'll change it so that it does that based on what you mentioned with addling the system locale!

@micahmo
Copy link
Member

micahmo commented Mar 8, 2024

I'll change it so that it does that based on what you mentioned with addling the system locale!

Sorry for commenting so much on this one. One more thought to add is that, if the system format happens to be a duplicate of one of the predefined formats, you could hide that so it's only displayed once. You can see what the system format would be by checking pattern.

'${dateFormat.pattern} ${timeFormat.pattern}'

@hjiangsu
Copy link
Member Author

hjiangsu commented Mar 8, 2024

Alright, I made some slight changes based on the feedback!

  • When the full date is shown, it shows up as slightly dimmer on the feed page
  • By default, the full date will use the system's locale to determine the format of the date
  • Removed any duplicated formats if it exists (53184a5)
  • I also removed the seconds from the date. My reason for this is that seconds don't contribute as much information as the rest of the timestamp, but takes up horizontal real-estate which makes it feel "clunky"

Here's an updated video with the changes. Let me know what you think!

81fc8c04-2868-417c-a743-9eb8ee049dba.mp4

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome feature and nice improvement from feedback!

@hjiangsu
Copy link
Member Author

I'll go ahead and merge this in so that I can push this as part of the next nightly!

@hjiangsu hjiangsu merged commit 9fbd236 into develop Mar 10, 2024
1 check passed
@hjiangsu hjiangsu deleted the feature/full-post-date branch March 10, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants