-
-
Notifications
You must be signed in to change notification settings - Fork 79
Date format: expose setting #2610
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are conflicts so I haven't tested but this code looks good to me.
# Conflicts fixed: # libcore/FileUtils.vala
Remove leftover code Co-authored-by: Ryo Nakano <[email protected]>
Remove extraneous space Co-authored-by: Ryo Nakano <[email protected]>
Remove extraneous space Co-authored-by: Ryo Nakano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving one optional comment but otherwise LGTM and works as expected.
src/View/Widgets/AppMenu.vala
Outdated
locale_button.get_style_context ().add_class (Gtk.STYLE_CLASS_MENUITEM); | ||
var informal_button = new Gtk.RadioButton.with_label_from_widget (iso_button, DateFormatMode.INFORMAL.to_string ()); | ||
informal_button.get_style_context ().add_class (Gtk.STYLE_CLASS_MENUITEM); | ||
//TODO Add a custom format wizard? Or a detailed informat format? Or "days ago" format? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should simply remove this TODO and file an issue if we won't address it in this PR but really want to in the future. TODO comments are often buried in projects that have many source codes and are developed by multiple developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've now limited this PR to exposing the setting. We can consider other formats later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Any setting that is actively supported should be accessible through the UI. A user could have reason to change the datetime format for a particular folder/purpose.
For now, the existing three options are exposed as radiobuttons appended to the AppMenu.