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

Too little spacing for the texts of the header links #12277

Closed
nickvergessen opened this issue Nov 5, 2018 · 16 comments
Closed

Too little spacing for the texts of the header links #12277

nickvergessen opened this issue Nov 5, 2018 · 16 comments
Assignees
Labels
bug design Design, UI, UX, etc. regression
Milestone

Comments

@nickvergessen
Copy link
Member

bildschirmfoto von 2018-11-05 13-37-53

Also shorting the words to "Annou…" for announcements and other similar things, without having a hover fallback looks weird to me.

@nextcloud/designers

@nickvergessen nickvergessen added bug design Design, UI, UX, etc. regression labels Nov 5, 2018
@nickvergessen nickvergessen added this to the Nextcloud 15 milestone Nov 5, 2018
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #9316 (Add Support for Link headers), #2693 (More space for navigation text), #11502 (Fix share header text on small widths), #10024 (Footer link to text (Theming App)), and #11511 ([stable14] Fix share header text on small widths).

@jancborchardt
Copy link
Member

Right, of course this drawback would appear.

So in the previous version we had full title visibility for the currently hovered/focused element. It would probably fix the issue if the currently selected element’s text gets as much width as the text needs, and the entries left and right are truncated accordingly. Not sure this is easily possible but I have some ideas with either background-color or :after / :before and padding-left/right.

@jancborchardt
Copy link
Member

I tried this but it just doesn’t look good and feels janky, and is not flexible (the overflow-y: hidden even needs to be removed from the li, causing the glitching again):

			/* Show full app title for hovered app */
			&:hover span {
				width: 150%;
				transform: translateX(-17%);
				background-color: var(--color-primary);
				z-index: 10;
			}

Other ideas @nextcloud/designers?

@skjnldsv
Copy link
Member

skjnldsv commented Nov 6, 2018

I would just add a 2px margin on the span and set the width to calc(100% - 4px) 🤔

capture d ecran_2018-11-05_16-52-01

@jancborchardt
Copy link
Member

@skjnldsv it’s ok, but it makes the other issue @nickvergessen mentioned even worse cause it shows less characters:

Also shorting the words to "Annou…" for announcements and other similar things, without having a hover fallback looks weird to me.

Like "Bookm…" is now "Book…". So I think both the issue of spacing and the issue of showing the full name on hover/focus need to be addressed?

Do you have any ideas, or improvements to what I commented above at #12277 (comment) ?

@skjnldsv
Copy link
Member

skjnldsv commented Nov 7, 2018

Yep, it's fixing an issue but not improving the other ^^
I'm fine with that tbh. Or else i'll bring back marquee and we'll be doomed for eternity! 😈

@juliusknorr
Copy link
Member

@skjnldsv You seem to be pretty focused on bringing marquee into Nextcloud. How about building an app for that? 😜

@skjnldsv
Copy link
Member

skjnldsv commented Nov 7, 2018

I miss it 😿

@nickvergessen
Copy link
Member Author

Anyhow we need a fix for this before 15 🙈

@skjnldsv
Copy link
Member

skjnldsv commented Nov 7, 2018

Anyhow we need a fix for this before 15

What is your opinion on what's above? :)

@nickvergessen
Copy link
Member Author

well more padding yes, I think showing the full name on hover is a good option, I dislike marquee, but well as long as it looks better than now, im all for it.

@skjnldsv
Copy link
Member

skjnldsv commented Nov 8, 2018

I dislike marquee

I was sarcastic :p

@MorrisJobke
Copy link
Member

What to do here? We should look into this, because on Thursday is the first RC

@jancborchardt
Copy link
Member

@skjnldsv do we want to combine both our approaches? Having a bit more padding by default first, but then on hover showing the complete name of the currently hovered app?

@skjnldsv
Copy link
Member

Seems good @jancborchardt 👍 😉

@jancborchardt
Copy link
Member

@skjnldsv ok, can you start with a pull request with the approach you used above? That definitely fixes the spacing. Then I will add the work-in-progress of what I listed and we see if we can make it look good. If not, we just take your commit. Sound good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug design Design, UI, UX, etc. regression
Projects
None yet
Development

No branches or pull requests

6 participants