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

Fixes more apps menu and settings/user menu positions #10723

Merged
merged 3 commits into from
Aug 20, 2018

Conversation

weeman1337
Copy link
Member

  • Sets the more apps menu container a width of 0. There is now no risk to catch clicks.
    image

closes #10716

  • Also tweaks the settings/user menu triangle position

closes #10722

Tested in latest Chrome, Edge, Firefox and Safari.

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
@weeman1337 weeman1337 added bug design Design, UI, UX, etc. 3. to review Waiting for reviews labels Aug 16, 2018
@weeman1337 weeman1337 added this to the Nextcloud 14 milestone Aug 16, 2018
blizzz
blizzz previously approved these changes Aug 17, 2018
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

⚫🧙 ;) works and looks good. No idea whether it could have other implications, but on first sight all looks fine.

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

To fix the settings menu, this property needs to be !important or have a higher selector weight :)

right: 17px;

See other comments otherwise! ;)
Thanks again for taking care of this!

@@ -98,7 +98,7 @@
width: 0;
position: absolute;
pointer-events: none;
right: 10px;
right: 22px;
Copy link
Member

Choose a reason for hiding this comment

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

This is breaking the other menus.
capture d ecran_2018-08-17_15-57-15

Copy link
Member Author

Choose a reason for hiding this comment

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

Good finding!

@@ -226,9 +226,10 @@
/* NAVIGATION --------------------------------------------------------------- */
nav[role='navigation'] {
display: inline-block;
width: $header-height;
width: 0;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not fond of hacks like that, I could prefer a proper :

li#more-apps {
	z-index: 2;
}

:)

@blizzz blizzz dismissed their stale review August 17, 2018 14:20

I better be silent about CSS :)

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
@@ -400,6 +400,10 @@ nav[role='navigation'] {
display: block;
}
}

#expanddiv:after {
right: 22px;
Copy link
Member

@skjnldsv skjnldsv Aug 17, 2018

Choose a reason for hiding this comment

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

This or

server/core/css/header.scss

Lines 406 to 409 in ccb8f7d

#expanddiv {
&.menu {
right: 17px;
}

Pick one ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

✓ done

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings/user menu misaligned More apps menu not position in the right place
4 participants