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

Fix settings papercuts #115

Merged
merged 2 commits into from
Aug 8, 2018
Merged

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Aug 6, 2018

From nextcloud/server#10094:

  • External sites: 3-dot-menu contents are shown by default, should not be.
    Also, it should be in a popover amd not just below.
    Id vote against moving all those additional options to a popover menu, since that would be huge and the current view is quite clear. @nextcloud/designers
  • External sites: No proper confirmation when a site is saved, and does not immediately show in header
    Fixed by using the rebuildNavigation method from the app settings. It is only this function that is still left in settings/js/apps.js, so it is not that much addition js anymore.
  • External sites: settings.svg is only available in dark, and is the same as the "external" icon?
    @nickvergessen Maybe you can tell if there is a reason, it is the same icon?

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@nickvergessen
Copy link
Member

nickvergessen commented Aug 8, 2018

External sites: settings.svg is only available in dark, and is the same as the "external" icon?

The thing is, the settings menu is white, so if you use the white external-icon its invisible. So I added a black one which has a different name. Also back in the days there was a problem with the size (icons in the settings were still the size of the header), so the settings icon is 16*16 and the external one is 32*32

@nickvergessen nickvergessen merged commit 23056c9 into master Aug 8, 2018
@nickvergessen nickvergessen deleted the bugfix/noid/navigation-update branch August 8, 2018 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants