GNOME Bugzilla – Bug 744817
Implement new calendar design
Last modified: 2015-02-20 22:03:57 UTC
Next step (after bug 744815) in the notification redesign - this implements the calendar/non-banner related bits. Patches are on top of wip/sass, and available in wip/fmuellner/notification-redux+sass.
Created attachment 297316 [details] [review] Revert "calendar: Use Clutter.GridLayout" GridLayout has some weird allocation issues, TableLayout - deprecated or not - works, so just keep using that for now. This reverts commit e9f95ca605c225b97963b7a0d5df3dfc6b4702da.
Created attachment 297317 [details] [review] dateMenu: Swap calendar and event columns The new design has the events list on the left and the calendar on the right, so swap them around and remove the vertical separator between them in favor of some additional whitespace as in the mockups.
Created attachment 297318 [details] [review] dateMenu: Make "today" button more prominent Split the weekday from the date and increase the latter's size to match the latest calendar mockups.
Created attachment 297319 [details] [review] dateMenu: Remove link to Date & Time Settings Menu items are on their way out of the calendar drop down, so let's start with the easy one. This one is removed without replacement, but then the Date & Time panel should be a one-time stop for most users anyway, so not having a direct shortcut should not be much of a problem. It is also the last remaining Settings item outside the system menu ...
Created attachment 297320 [details] [review] panel: Add function to close the calendar We will soon replace regular menu items in the calendar drop-down with more complex elements. However there will still be items that should close the drop down when activated - rather than making the menu available throughout the hierarchy (and eventually from outside as well when we add the notifications list), have a public method on a global object just like the ubiquitous Main.overview.hide().
Created attachment 297321 [details] [review] calendar: Copy URLHighlighter and friends here We will start using both URLHighlighter and the _fixMarkup() helper method the same way it's used in MessageTray. Usually we should make fixMarkup() public and call the existing methods, but we are planning for them to go away soon, so just keep two copies until the original one is removed.
Created attachment 297322 [details] [review] calendar: Add MessageList and Section/Message base types The message list is a scrollable list that will hold sections of different types of time-related messages like notifications, calendar events or birthday reminders. When no section displays any content for the selected date, a placeholder is shown instead.
Created attachment 297323 [details] [review] dateMenu: Replace EventsList with MessageList Turn the existing EventsList into a MessageListSection and add the message list to the calendar drop-down. The new events list only displays events for the currently selected day, but in a more structured and friendlier way than the old one.
Created attachment 297324 [details] [review] dateMenu: Remove "Open Calendar" item This functionality is now available through the Events section header in the message list, so we don't need a separate menu item for it.
Created attachment 297325 [details] [review] dateMenu: Add world clock section Rather than just offering to open Clocks when installed, pick up configured locations and display their time directly in the popup.
Created attachment 297326 [details] [review] dateMenu: Stop hiding the events list The message list is not only a replacement for the calendar events list, it will also take over the notification summary from the message tray. As we start drawing events from other sources than calendars, hiding it based on whether or not any calendars have been set up is no longer appropriate, so always include it in the calendar drop-down now.
Created attachment 297327 [details] [review] dateMenu: Freeze popup size while browsing As the popup's height depends on its content, which itself varies depending on the selected day, browsing the calendar can result in distracting size changes. To avoid this, the design calls for the height to be frozen to the previous one in that case. As the popup will always open with the current day selected, we don't have to be very sophisticated and can just lock the popup to the height corresponding to that day.
Created attachment 297328 [details] [review] messageTray: Make Notification._onClicked handler public When we will start to show notifications in the date drop-down, we will not use the actual notification actor, but construct our own UI based on Calendar.Message. This is similar to what we already do in the lock screen, except that in this case clicking the notification should activate the default action. So rename the existing _onClicked() method to activate() to make it clear that such use is acceptable. While not strictly necessary, also rename the corresponding signal to match.
Created attachment 297329 [details] [review] messageTray: Notify on notification updates Since notification support was added to the lock screen, notifications are no longer necessarily represented by the actual notification actor anymore. However when an existing notification is updated, external representations currently become outdated. Emit an appropriate signal which allows them to update.
Created attachment 297330 [details] [review] calendar: Add NotificationSection to message list Display notifications that have not been dismissed in the message list - eventually this will replace the existing message tray summary. Notification messages show icon, title and one line of the body and can be clicked to activate the default action. However they cannot be expanded, so other actions or the full body text are not accessible in this mode.
Created attachment 297331 [details] [review] notificationDaemon: Close calendar when opening app Just like we leave the overview when activating a notification, the calendar popup should be closed - after all, the only case where the calendar is open when a notification's default action is activated is the user clicking an entry in the message list's notification section.
Created attachment 297332 [details] [review] notificationDaemon: Do the same for GTK+ notifications There is not good reason why activating a GTK+ notification should behave fundamentally different from fd.o notifications - we don't raise the app because we expect it to perform an appropriate action, but that does not include closing overview or calendar for us ...
Review of attachment 297316 [details] [review]: OK
Review of attachment 297317 [details] [review]: OK
Review of attachment 297318 [details] [review]: OK
Review of attachment 297319 [details] [review]: OK
Review of attachment 297321 [details] [review]: OK
Review of attachment 297322 [details] [review]: ::: js/ui/calendar.js @@ +1086,3 @@ + })); + + okay.... where is used? @@ +1121,3 @@ + + clear: function() { + }, is ... on pourpose? if yes, skip this comment.
Review of attachment 297320 [details] [review]: ok
(In reply to Carlos Soriano from comment #24) > ::: js/ui/calendar.js > @@ +1086,3 @@ > okay.... where is used? I did use it originally to keep urgent notifications on top of the list, but I figured out a different way to do that. So indeed this can be removed now \o/ > @@ +1121,3 @@ > is ... on pourpose? if yes, skip this comment. The ... is the spread[0] operator, so yeah, completely on purpose. [0] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator
Review of attachment 297323 [details] [review]: free to push after clarify/fix =) ::: js/ui/calendar.js @@ +1256,3 @@ + let app = this._getCalendarApp(); + if (app.get_id() == 'evolution.desktop') + app = Gio.DesktopAppInfo.new('evolution-calendar.desktop'); Why evolution is special? @@ +1293,3 @@ this.actor.add_actor(this._icon); + this._label = new St.Label({ text: _("No Events") }); ups? an error before?
Review of attachment 297324 [details] [review]: ok
(In reply to Carlos Soriano from comment #28) > @@ +1256,3 @@ > + let app = this._getCalendarApp(); > + if (app.get_id() == 'evolution.desktop') > + app = Gio.DesktopAppInfo.new('evolution-calendar.desktop'); > > Why evolution is special? Because. Evolution's .desktop file launches with the last-active component, which is clearly not what we want ("You clicked 'Open Calendar'? How about some mail instead?"). The 'evolution-calendar' .desktop file we launch instead is actually a hidden one we ship ourselves (see src/calendar-server) to work around this and still get startup notifications. Bug 677907 has some of the context ... > @@ +1293,3 @@ > this.actor.add_actor(this._icon); > > + this._label = new St.Label({ text: _("No Events") }); > > ups? an error before? Yup, thanks for the catch!
I see lots of "accepted-commit_now" here on patches though we are under UI freeze? Should be "accepted-commit_after_freeze" I guess.
Well, the freeze is considered in action after .90 tarballs (and tarballs are expected on Monday, and we are on Friday, and many things are still being reviewed and pushed in gnome-shell, meh.).
Review of attachment 297325 [details] [review]: free to push althought my comment ::: js/ui/dateMenu.js @@ +107,3 @@ + let layout = new Clutter.GridLayout({ orientation: Clutter.Orientation.VERTICAL }); + this._grid = new St.Widget({ style_class: 'world-clocks-grid', + x_fill: true, I would divide this in two. The header static, and then a scroll view with a gridlayout. So if you need to use the scroll view, the header stay visible. Something like Button -> BoxLayout(vertical:true) -> Header, GridLayout -> clocks it is a corner case, but that is for what we use a scrollview in the parent, and this invalidates it a little this use case. But is ok for now, good for a future work...
Review of attachment 297326 [details] [review]: OK
Review of attachment 297327 [details] [review]: nice touch!
(In reply to Carlos Soriano from comment #33) > I would divide this in two. The header static, and then a scroll view with a > gridlayout. So if you need to use the scroll view, the header stay visible. I would agree if the design only included the world clock there, but more will be added there in the future (see https://wiki.gnome.org/Design/OS/Notifications/Redux). The design explicitly says: "If the number of world clocks exceeds the space available, the section containing weather and world clocks scrolls." So when weather is added in the future, it should be added to the same scroll view as clocks.
Review of attachment 297328 [details] [review]: yep
Review of attachment 297329 [details] [review]: OK
Review of attachment 297330 [details] [review]: oh, so they can be expanded in normal mode, but not here? I guess that's by design right? Looks odd to me. We can discuss that after releasing 3.15.90 I guess
Review of attachment 297331 [details] [review]: OK
Review of attachment 297332 [details] [review]: Yep!
yay!
Attachment 297316 [details] pushed as 39cfe48 - Revert "calendar: Use Clutter.GridLayout" Attachment 297317 [details] pushed as 26330fd - dateMenu: Swap calendar and event columns Attachment 297318 [details] pushed as 33c5f57 - dateMenu: Make "today" button more prominent Attachment 297319 [details] pushed as de27e4e - dateMenu: Remove link to Date & Time Settings Attachment 297320 [details] pushed as f84addc - panel: Add function to close the calendar Attachment 297321 [details] pushed as 053e54f - calendar: Copy URLHighlighter and friends here Attachment 297322 [details] pushed as 464f552 - calendar: Add MessageList and Section/Message base types Attachment 297323 [details] pushed as e850d9e - dateMenu: Replace EventsList with MessageList Attachment 297324 [details] pushed as 7d1382a - dateMenu: Remove "Open Calendar" item Attachment 297325 [details] pushed as efc0ec4 - dateMenu: Add world clock section Attachment 297326 [details] pushed as 88c7039 - dateMenu: Stop hiding the events list Attachment 297327 [details] pushed as e404369 - dateMenu: Freeze popup size while browsing Attachment 297328 [details] pushed as 65a5baa - messageTray: Make Notification._onClicked handler public Attachment 297329 [details] pushed as 025985e - messageTray: Notify on notification updates Attachment 297330 [details] pushed as dfd8870 - calendar: Add NotificationSection to message list Attachment 297331 [details] pushed as 11b3ed2 - notificationDaemon: Close calendar when opening app Attachment 297332 [details] pushed as b00f122 - notificationDaemon: Do the same for GTK+ notifications
*** Bug 744110 has been marked as a duplicate of this bug. ***