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.

Sign up for GitHub

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

Jump to bottom

Fix navigation layout for items that are added on a re-render #12481

Merged
merged 1 commit into from
Nov 16, 2018
Merged

Fix navigation layout for items that are added on a re-render #12481

merged 1 commit into from
Nov 16, 2018

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Nov 16, 2018 •
edited by MorrisJobke
Loading

Bug was not visible until #12153
Now it looks awkward with the externals app

bildschirmfoto von 2018-11-16 10-54-33

Fixes #12464

@nickvergessen nickvergessen added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: settings labels Nov 16, 2018
@nickvergessen nickvergessen added this to the Nextcloud 15 milestone Nov 16, 2018
@skjnldsv
Copy link
Member

Fix #12464 ?

@MorrisJobke
Copy link
Member

Fix #12464 ?

Already added ;)

@MorrisJobke
Copy link
Member

I don't get it. For me the code is locally executed but the result still looks wrong:

bildschirmfoto 2018-11-16 um 11 12 59

I refreshed multiple times, reinstalled, used different browsers ... and still - the newly added apps are still rendered wrongly.

So this does not fix #12481 somehow. for me.

skjnldsv
skjnldsv requested changes Nov 16, 2018
View reviewed changes
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.

Let me fix that

@skjnldsv skjnldsv force-pushed the bugfix/noid/fix-navigation-layout-of-new-items branch from 10cd800 to 4604a74 Compare November 16, 2018 11:06
skjnldsv
skjnldsv approved these changes Nov 16, 2018
View reviewed changes
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.

Done.
The span is next to the a, not within it.
I also fixed the size of the apps icons that did not reflect what the php was initially outputting.

@nickvergessen @skjnldsv
Fix navigation layout for items that are added on a re-render
44b0d39 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@skjnldsv skjnldsv force-pushed the bugfix/noid/fix-navigation-layout-of-new-items branch from 4604a74 to 44b0d39 Compare November 16, 2018 11:08
MorrisJobke
MorrisJobke approved these changes Nov 16, 2018
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 56fdb22 into master Nov 16, 2018
@MorrisJobke MorrisJobke deleted the bugfix/noid/fix-navigation-layout-of-new-items branch November 16, 2018 12:39
@MorrisJobke MorrisJobke mentioned this pull request Nov 22, 2018
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@rullzer rullzer rullzer approved these changes

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug design Design, UI, UX, etc. feature: settings
Projects
None yet
Milestone
Nextcloud 15
Development

Successfully merging this pull request may close these issues.
4 participants
@nickvergessen @skjnldsv @MorrisJobke @rullzer