Use flexbox for header and rearrange some elements #2924

ChristophWurst · 2017-01-03T16:28:31Z

this is just a little cleanup for #207

mention-bot · 2017-01-03T16:28:34Z

@ChristophWurst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jancborchardt, @Henni and @nickvergessen to be potential reviewers.

ChristophWurst · 2017-01-03T16:30:09Z

core/css/styles.css

@@ -125,7 +124,6 @@ body {
 .searchbox input[type="search"]:valid {
 	color: #fff;
 	width: 155px;
-	max-width: 50%;


@skjnldsv if this line is active, the input is narrower on the right, which means some space between the input and the settings menu shows ups. Any idea what might cause that? I've tried different fixes but none solved it so far. The max-width is needed for narrow window sizes like on mobile to prevent the input take up too much space.

Will look into it asap

Almost have it. Will push it tonight!

nickvergessen · 2017-01-05T13:00:21Z

Did you try if this breaks the notification UI?

ChristophWurst · 2017-01-05T13:45:12Z

Did you try if this breaks the notification UI?

Not yet, thanks for the hint

skjnldsv · 2017-01-05T17:58:26Z

@ChristophWurst Pushed what I got so far.
You can now tweak the input width to fit. The issue is coming from it.

ChristophWurst · 2017-01-09T13:05:11Z

Awesome, thank you @skjnldsv!

I added another small fix. Now it seems to work nicely, except for extremely narrow window sizes. There, the fixed search input width is too big.

ChristophWurst · 2017-01-09T13:18:53Z

@nickvergessen apparently works with notifications too:

ChristophWurst · 2017-01-09T13:23:04Z

I added another small fix. Now it seems to work nicely, except for extremely narrow window sizes. There, the fixed search input width is too big.

@eppfel @jancborchardt @juliushaertl @skjnldsv not sure how much of an issue that is, but would be great if we can fix this too. Suggestions are very welcome :-)

nickvergessen · 2017-01-09T13:29:16Z

Previously the notification icon was right of the search, but I guess it's okay

ChristophWurst · 2017-01-09T13:43:24Z

Previously the notification icon was right of the search, but I guess it's okay

We can change that of course. https://github.com/nextcloud/notifications/blob/643b3d8e26a821a2b5257064f9b9d8804e8e044d/js/app.js#L93-L94 modifies the relevant elements.

codecov-io · 2017-01-09T13:49:34Z

Current coverage is 53.92% (diff: 0.00%)

Merging #2924 into master will not change coverage

@@             master      #2924   diff @@
==========================================
  Files          1302       1302          
  Lines         80097      80097          
  Methods        7905       7905          
  Messages          0          0          
  Branches       1245       1245          
==========================================
  Hits          43189      43189          
  Misses        36908      36908          
  Partials          0          0

Diff Coverage	File Path
0%	core/templates/layout.user.php
0%	apps/files_sharing/templates/public.php

Powered by Codecov. Last update 250b711...db5f164

jancborchardt · 2017-01-14T16:13:03Z

core/templates/layout.user.php

-					</h1>
-				</div>
-			</a>
+			<div class="header-left">


You use an ID for header-right, but a class here. Better would be an ID here too. :)

jancborchardt · 2017-01-14T16:14:02Z

core/css/header.scss

@@ -414,6 +412,20 @@
 /* do not show display name when profile picture is present */

 #header {
+	#header-left #header-right {


Comma and line break missing I guess? Because header-right is a sibling of header-left, and not a child.

jancborchardt · 2017-01-14T16:14:32Z

core/css/header.scss

+		display: flex;
+	}
+
+	div.header-left {


As with the comment below, use an ID. Then also remove the div part because an ID (or class) is enough to identify. :)

jancborchardt · 2017-01-14T16:15:49Z

@ChristophWurst @skjnldsv see my detail comments. Except if this is intentional of course, but it doesn’t look like it :)

fixed

skjnldsv

The search icon is a little too low.

skjnldsv · 2017-01-17T07:34:29Z

core/css/styles.scss

@@ -169,7 +169,6 @@ body {
 	border: 0;
 	border-radius: 3px;
 	margin-top: 9px;


Search box is too low.
Remove the marginTop

skjnldsv · 2017-01-17T08:43:33Z

Very nice work!
Glad to see we're improving the css a lot! ;)

ChristophWurst · 2017-01-17T08:58:47Z

I will take care of fixing Gallery in case this breaks something there too

jancborchardt · 2017-01-17T15:18:27Z

Gallery is also effected btw 🙌

Video calls shared by link will be affected too ;) And maybe Collabora Office uses a separate template for the header too?

ChristophWurst · 2017-01-17T19:50:18Z

Video calls shared by link will be affected too ;) And maybe Collabora Office uses a separate template for the header too?

then someone will fix it ;-)

MorrisJobke

I rebased this, resolved the conflicts, tested it and it works like a charm 👍

@ChristophWurst Could you please check that the rebase went fine (otherwise db5f164 is the old sha sum ;))

skjnldsv · 2017-01-19T05:37:10Z

@MorrisJobke 😍

ChristophWurst · 2017-01-19T09:37:47Z

I rebased this, resolved the conflicts, tested it and it works like a charm 👍

❤️ 😗 thanks :-)

Will check it ASAP

Signed-off-by: Christoph Wurst <[email protected]>

ChristophWurst · 2017-01-23T08:12:10Z

rebased again because it was conflicting with the changes from #3187

New issues:

~~search input lost its border~~ -> not an issue of this PR Search box: background flashes white when clicked and box has no border #3205
~~public page broke completely~~ issue of master too:

cc @nextcloud/designers

MorrisJobke

I tested this again and it looks fine 👍

MorrisJobke · 2017-01-23T18:16:12Z

It will only need a little adjustment of the notification icon in the notifications app. I will do that.

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke · 2017-01-23T18:18:41Z

It will only need a little adjustment of the notification icon in the notifications app. I will do that.

nextcloud/notifications#58

ChristophWurst added 2. developing Work in progress design Design, UI, UX, etc. labels Jan 3, 2017

ChristophWurst added this to the Nextcloud 12.0 milestone Jan 3, 2017

ChristophWurst commented Jan 3, 2017

View reviewed changes

ChristophWurst force-pushed the flexbox-header branch from 911db1e to 237de91 Compare January 3, 2017 16:30

ChristophWurst force-pushed the flexbox-header branch from 90c02dd to a846327 Compare January 9, 2017 12:47

ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 9, 2017

ChristophWurst requested review from jancborchardt, eppfel and juliusknorr January 9, 2017 13:21

ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 9, 2017

jancborchardt previously requested changes Jan 14, 2017

View reviewed changes

ChristophWurst force-pushed the flexbox-header branch from 83b39b9 to f25c09e Compare January 16, 2017 13:31

ChristophWurst requested review from rullzer, nickvergessen and MorrisJobke January 16, 2017 13:58

skjnldsv requested changes Jan 17, 2017

View reviewed changes

ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 17, 2017

skjnldsv approved these changes Jan 17, 2017

View reviewed changes

jancborchardt mentioned this pull request Jan 17, 2017

Header scss optimisation #3050

Merged

MorrisJobke force-pushed the flexbox-header branch from db5f164 to d1f3fc2 Compare January 19, 2017 05:31

MorrisJobke approved these changes Jan 19, 2017

View reviewed changes

ChristophWurst added 8 commits January 23, 2017 09:03

Use flexbox for header and rearrange some elements

2025092

Signed-off-by: Christoph Wurst <[email protected]>

use less space for the left part of the header

b590658

Signed-off-by: Christoph Wurst <[email protected]>

fix scss

7c824a6

Signed-off-by: Christoph Wurst <[email protected]>

more cleanup

bfa50a4

Signed-off-by: Christoph Wurst <[email protected]>

fix log and app name

a1f9425

Signed-off-by: Christoph Wurst <[email protected]>

fix public sharing buttons

47c63e6

Signed-off-by: Christoph Wurst <[email protected]>

fix searchbox position

bfc4590

Signed-off-by: Christoph Wurst <[email protected]>

fix appname position

ac6d501

Signed-off-by: Christoph Wurst <[email protected]>

ChristophWurst force-pushed the flexbox-header branch from d1f3fc2 to ac6d501 Compare January 23, 2017 08:08

MorrisJobke approved these changes Jan 23, 2017

View reviewed changes

MorrisJobke merged commit d80bc6c into master Jan 23, 2017

MorrisJobke deleted the flexbox-header branch January 23, 2017 18:16

MorrisJobke added a commit to nextcloud/notifications that referenced this pull request Jan 23, 2017

Fix sizing of the icon for nextcloud/server#2924

9d628f6

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke mentioned this pull request Jan 23, 2017

Fix sizing of the icon nextcloud/notifications#58

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use flexbox for header and rearrange some elements #2924

Use flexbox for header and rearrange some elements #2924

ChristophWurst commented Jan 3, 2017

mention-bot commented Jan 3, 2017

ChristophWurst Jan 3, 2017

skjnldsv Jan 3, 2017

skjnldsv Jan 5, 2017

nickvergessen commented Jan 5, 2017

ChristophWurst commented Jan 5, 2017

skjnldsv commented Jan 5, 2017 •

edited

Loading

ChristophWurst commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

nickvergessen commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

codecov-io commented Jan 9, 2017 •

edited

Loading

jancborchardt Jan 14, 2017

jancborchardt Jan 14, 2017

jancborchardt Jan 14, 2017

jancborchardt commented Jan 14, 2017

skjnldsv left a comment •

edited

Loading

skjnldsv Jan 17, 2017 •

edited

Loading

ChristophWurst Jan 17, 2017

skjnldsv commented Jan 17, 2017

ChristophWurst commented Jan 17, 2017

jancborchardt commented Jan 17, 2017

ChristophWurst commented Jan 17, 2017

MorrisJobke left a comment

skjnldsv commented Jan 19, 2017

ChristophWurst commented Jan 19, 2017

ChristophWurst commented Jan 23, 2017 •

edited

Loading

MorrisJobke left a comment

MorrisJobke commented Jan 23, 2017

MorrisJobke commented Jan 23, 2017

Use flexbox for header and rearrange some elements #2924

Use flexbox for header and rearrange some elements #2924

Conversation

ChristophWurst commented Jan 3, 2017

mention-bot commented Jan 3, 2017

ChristophWurst Jan 3, 2017

Choose a reason for hiding this comment

skjnldsv Jan 3, 2017

Choose a reason for hiding this comment

skjnldsv Jan 5, 2017

Choose a reason for hiding this comment

nickvergessen commented Jan 5, 2017

ChristophWurst commented Jan 5, 2017

skjnldsv commented Jan 5, 2017 • edited Loading

ChristophWurst commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

nickvergessen commented Jan 9, 2017

ChristophWurst commented Jan 9, 2017

codecov-io commented Jan 9, 2017 • edited Loading

Current coverage is 53.92% (diff: 0.00%)

jancborchardt Jan 14, 2017

Choose a reason for hiding this comment

jancborchardt Jan 14, 2017

Choose a reason for hiding this comment

jancborchardt Jan 14, 2017

Choose a reason for hiding this comment

jancborchardt commented Jan 14, 2017

skjnldsv left a comment • edited Loading

Choose a reason for hiding this comment

skjnldsv Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

ChristophWurst Jan 17, 2017

Choose a reason for hiding this comment

skjnldsv commented Jan 17, 2017

ChristophWurst commented Jan 17, 2017

jancborchardt commented Jan 17, 2017

ChristophWurst commented Jan 17, 2017

MorrisJobke left a comment

Choose a reason for hiding this comment

skjnldsv commented Jan 19, 2017

ChristophWurst commented Jan 19, 2017

ChristophWurst commented Jan 23, 2017 • edited Loading

MorrisJobke left a comment

Choose a reason for hiding this comment

MorrisJobke commented Jan 23, 2017

MorrisJobke commented Jan 23, 2017

skjnldsv commented Jan 5, 2017 •

edited

Loading

codecov-io commented Jan 9, 2017 •

edited

Loading

skjnldsv left a comment •

edited

Loading

skjnldsv Jan 17, 2017 •

edited

Loading

ChristophWurst commented Jan 23, 2017 •

edited

Loading