Optimize header #4102

michaelletzgus · 2017-03-27T19:07:40Z

Idea: Stop notification from overlapping everything else:

Optimize header, introduce header-center, place notifications in it
Please ignore colors, will be removed in final version

Is this something worth finishing?

codecov-io · 2017-03-27T19:43:38Z

Codecov Report

Merging #4102 into master will decrease coverage by 0.14%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #4102      +/-   ##
============================================
- Coverage     54.18%   54.03%   -0.15%     
  Complexity    21640    21640              
============================================
  Files          1327     1268      -59     
  Lines         82791    75527    -7264     
  Branches       1312        0    -1312     
============================================
- Hits          44859    40810    -4049     
+ Misses        37932    34717    -3215

Impacted Files	Coverage Δ	Complexity Δ
core/templates/layout.user.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
apps/files_sharing/templates/public.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
apps/comments/lib/EventHandler.php	`79.16% <0%> (-8.34%)`	`7% <0%> (ø)`
lib/private/User/Manager.php	`83.81% <0%> (-0.58%)`	`68% <0%> (ø)`
core/js/sharedialoglinkshareview.js
core/js/config.js
core/js/files/client.js
settings/js/apps.js
core/js/sharesocialmanager.js
core/js/oc-requesttoken.js
... and 55 more

MorrisJobke · 2017-03-27T23:41:02Z

The notifications doesn't appear anymore when I execute: OC.Notification.show('Hello - I\'m a notification') - work fine on master. Could you check that?

jancborchardt · 2017-03-27T23:59:28Z

@michaelletzgus didn’t check yet but it’s a good initiative! cc @nextcloud/designers for review.

@michaelletzgus please make sure that it also works well on mobile (narrow screens) and maybe also check out #301 (proposal for non-blocking notifications)

juliushaertl · 2017-03-28T16:15:02Z

I like the idea, but the issue with blocking UI should be fixed when touching the code as well:

Are those notifications being removed after some time? I'm not sure but I think that was already in somewhere in the past?

jancborchardt · 2017-03-28T17:14:12Z

Just checked and this will definitely create issues with the apps being in the header. It’s strange to push the app list away just to show a notification. It’s basically the same issue of overlaying again, but even worse :\

michaelletzgus · 2017-03-28T19:29:18Z

The current version in my branch is "under construction". My current goal is to create a (modal?) overlay
in the screen's center when space is limited.
Problem: WHEN exactly is space limited?
And: Why not use an popup-overlay always?

michaelletzgus · 2017-03-29T20:18:03Z

The notification bar now collapses into a screen centered popup at width<680.
(The CSS currently contains a small design flaw: The popup overlay is a child of header-center, will be fixed)

Question: Should ALL notifications be shown in a popup - even with wide screen? Or should they stay in the header?

MorrisJobke · 2017-03-29T21:40:02Z

core/js/js.js

-			});
-			$row.addClass('closeable');
-		}
+//		// add a close button


No need to double comment :D

MorrisJobke · 2017-03-29T21:40:32Z

core/templates/layout.user.php

+					$(document.createElement( "button" )).click(function(){ OC.Notification.show('Hello - I am a notification') }).appendTo($('#header-left')).html("B1");
+					$(document.createElement( "button" )).click(function(){ OC.Notification.showHtml('Hello - I <b>am</b> a very very <i>loooooong</i> <u>HTML noooooooooooootification</u>....................') }).appendTo($('#header-left')).html("B2");
+					$(document.createElement( "button" )).click(function(){ OC.Notification.showTemporary('temp. notification',{timeout:2}) }).appendTo($('#header-left')).html("B3");
+				</script>


Please remove debug code and just put it in the description. Everybody can then execute it themselves in the web dev console.

MorrisJobke

See comments

jancborchardt · 2017-03-29T22:17:56Z

Now the notifications are even more intrusive than before though. :\ And on the top they are not centered really.

To be honest, we want to get rid of most notifications in apps because they are just used as an easy way out but are not very nicely designed. It’s better to do it with .emptycontent or inline.

But maybe we find a nice way to display them in the header, maybe even in combination with the Notifications app https://github.com/nextcloud/notifications – where some of these notifications are maybe better put.

cc @nextcloud/designers

michaelletzgus · 2017-03-30T16:03:20Z

Of course it's not centered on screen because the free space in the header isn't.

jancborchardt · 2017-04-03T21:59:07Z

@michaelletzgus it would be a bit nicer to have a solution where the notification appears in a place which is consistent. It’s strange when it moves depending on other circumstances. Did you consider showing it below the header bar?

michaelletzgus · 2017-04-08T11:23:27Z

A consistent place would be nice - but below the header just moves the "invasive overlay problem" from the header a few pixels to the bottom.
I think a few questions should be answered before the redesign:

What is an important message?
Should important messages be invasive?
Are important very messages so important that they justify a modal overlay?
What are "notes", "hints" and "tips of the day"?

I think everything that can NOT be safely ignored (configuration problems, security problems, major (minor?) server updates, app updates are important and might be invasive/modal...

* Optimize header, introduce header-center, place notifications in it * Please ignore colors, will be removed in final version * Add buttons for testing * Always show the closing "x" * Center notification on small screens Signed-off-by: Michael Letzgus <michaelletzgus@users.noreply.github.com> kk

jancborchardt · 2017-04-25T12:36:33Z

What is an important message?

A new version being available for example.

Should important messages be invasive?

A bit, in the style that they are now, with a notification up top. They shouldn’t just be in the Notification app, should they @LukasReschke @nickvergessen?

Are important very messages so important that they justify a modal overlay?

Definitely not.

What are "notes", "hints" and "tips of the day"?

Anything like that should be displayed inline, not as a notification and especially not as a modal.

What do you think @nextcloud/designers?

nickvergessen · 2017-04-25T12:40:25Z

The header notifications are e.g. failed upload attempts via the web UI.
They should not be pushed to your desktop/mobile client, so not related to notifications app => unsubscribing

MorrisJobke · 2017-04-26T01:52:14Z

A new version being available for example.

I would not say, that this specific case is so important. A new version is fine to have in the notifications area.

I would only like stuff in this direct notification on the top for stuff that just happened. But the more I think about use cases the more it feels wrong to have this here. It's just a generic and ugly approach to handle feedback for the user, but actually it shouldn't be done like it is now.

Some examples:

undo user deletion: currently such an notification, but it should be shown where the user entry was before
failed to share a file because of REASON: this should be shown in the sharing sidebar - just there where you clicked to create the share
a new version is available: should be a real notification in our notification drop down and also be pushed to the clients, because this is an actual state of the server and not a state of the current session

The more I think about this, the more I think we should get rid of OC.Notifications and either use our notification center (for state of the instance, state of a user) or inline feedback/inline actions for stuff that just happened (like error handling and undo actions).

Opinions @nextcloud/designers ?

jancborchardt · 2017-04-26T13:22:50Z

@MorrisJobke yep, totally agree. As I said above:

To be honest, we want to get rid of most notifications in apps because they are just used as an easy way out but are not very nicely designed. It’s better to do it with .emptycontent or inline.

And everything which doesn’t work via emptycontent or inline, is probably best in the Notifications app (like a new version being available).

eppfel · 2017-05-04T22:05:19Z

So, we should therefore collect occasions where this nasty notification is still used and step by step remove it by putting custom notifications close to the context.

Closing, as it is a fix for a symptom, not for a cause. But thanks for kickstarting the discussion @michaelletzgus

michaelletzgus force-pushed the optinotifi branch from c53ed5b to 5cc0985 Compare March 28, 2017 15:59

michaelletzgus force-pushed the optinotifi branch from 2659a5f to 68c9d75 Compare March 28, 2017 16:40

rullzer added 2. developing Work in progress design Design, UI, UX, etc. labels Mar 28, 2017

michaelletzgus force-pushed the optinotifi branch 2 times, most recently from 98eb433 to 3540dbf Compare March 29, 2017 17:21

michaelletzgus assigned michaelletzgus and aroseartist and unassigned aroseartist Mar 29, 2017

michaelletzgus force-pushed the optinotifi branch from 3540dbf to a22fbc0 Compare March 29, 2017 21:04

MorrisJobke reviewed Mar 29, 2017

View reviewed changes

core/js/js.js

});

$row.addClass('closeable');

}

// // add a close button

Copy link

Member

MorrisJobke Mar 29, 2017

Choose a reason for hiding this comment

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

No need to double comment :D

MorrisJobke reviewed Mar 29, 2017

View reviewed changes

MorrisJobke requested changes Mar 29, 2017

View reviewed changes

jancborchardt added this to the Backlog milestone Mar 30, 2017

michaelletzgus force-pushed the optinotifi branch from a22fbc0 to 8e4f783 Compare March 30, 2017 16:37

michaelletzgus force-pushed the optinotifi branch 3 times, most recently from 17355fc to 2ca2e76 Compare April 8, 2017 10:22

michaelletzgus force-pushed the optinotifi branch from 2ca2e76 to fbf6a67 Compare April 13, 2017 15:07

michaelletzgus force-pushed the optinotifi branch 2 times, most recently from e589d0c to b5b4e0b Compare April 22, 2017 09:53

michaelletzgus force-pushed the optinotifi branch from b5b4e0b to 4f24a0a Compare April 23, 2017 14:11

MorrisJobke removed this from the Backlog milestone Apr 26, 2017

eppfel closed this May 4, 2017

MorrisJobke mentioned this pull request May 15, 2017

showNotification closes Gallery view (probably others too) #3504

Closed

michaelletzgus deleted the optinotifi branch June 24, 2017 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize header #4102

Optimize header #4102

michaelletzgus commented Mar 27, 2017 •

edited

Loading

codecov-io commented Mar 27, 2017 •

edited by codecov bot

Loading

MorrisJobke commented Mar 27, 2017

jancborchardt commented Mar 27, 2017

juliushaertl commented Mar 28, 2017

jancborchardt commented Mar 28, 2017

michaelletzgus commented Mar 28, 2017

michaelletzgus commented Mar 29, 2017 •

edited

Loading

MorrisJobke Mar 29, 2017

MorrisJobke Mar 29, 2017

MorrisJobke left a comment

jancborchardt commented Mar 29, 2017

michaelletzgus commented Mar 30, 2017

jancborchardt commented Apr 3, 2017

michaelletzgus commented Apr 8, 2017

jancborchardt commented Apr 25, 2017

nickvergessen commented Apr 25, 2017

MorrisJobke commented Apr 26, 2017

jancborchardt commented Apr 26, 2017

eppfel commented May 4, 2017

Optimize header #4102

Optimize header #4102

Conversation

michaelletzgus commented Mar 27, 2017 • edited Loading

codecov-io commented Mar 27, 2017 • edited by codecov bot Loading

Codecov Report

MorrisJobke commented Mar 27, 2017

jancborchardt commented Mar 27, 2017

juliushaertl commented Mar 28, 2017

jancborchardt commented Mar 28, 2017

michaelletzgus commented Mar 28, 2017

michaelletzgus commented Mar 29, 2017 • edited Loading

MorrisJobke Mar 29, 2017

Choose a reason for hiding this comment

MorrisJobke Mar 29, 2017

Choose a reason for hiding this comment

MorrisJobke left a comment

Choose a reason for hiding this comment

jancborchardt commented Mar 29, 2017

michaelletzgus commented Mar 30, 2017

jancborchardt commented Apr 3, 2017

michaelletzgus commented Apr 8, 2017

jancborchardt commented Apr 25, 2017

nickvergessen commented Apr 25, 2017

MorrisJobke commented Apr 26, 2017

jancborchardt commented Apr 26, 2017

eppfel commented May 4, 2017

michaelletzgus commented Mar 27, 2017 •

edited

Loading

codecov-io commented Mar 27, 2017 •

edited by codecov bot

Loading

michaelletzgus commented Mar 29, 2017 •

edited

Loading