-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
add a way to open the admin settings overview directly #33756
Conversation
Signed-off-by: szaimen <szaimen@e.mail.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! :)
We should also replace the icons with Material Design icons, but separate issue probably.
Then we can also use separate icons for the settings:
- Personal: person
- Admin: settings
But then it should be two separated pages, with each its own sidebar menu? If I understand the current PR both menu entry lead to the same page, just with a different default "tab"? |
Yes but that already makes a huge positive difference in my personal testing! |
Yes separate issuey however I could replace the personal settings icon with https://github.com/nextcloud/server/blob/master/apps/settings/img/personal.svg. WDYT? |
b9513a9
to
b2885aa
Compare
I visit the admin settings page should open the admin settings, not the personal settings of the admin, so I added a fixup commit for that; if the personal settings of the admin need to be opened a new I visit the personal settings page step should be added, but it is not currently needed, as all* the acceptance tests that open the admin settings do that to access an admin-level settings section. *Except the one in access-levels test, which just needs to open the settings page to check the available panels. |
fine by me as long as the tests pass :) BTW, is And can you please squash or shall I do it? |
db221b0
to
17ee17c
Compare
done |
@szaimen yes, that would be a good fix! :) |
Signed-off-by: szaimen <szaimen@e.mail.de>
done :) |
It seems so. Unfortunately I do not see why, and I locally run it 50 times and all of them passed, so... something for later ;-)
Thanks :-) |
CI failure unrelated |
This is best reviewed like this: https://github.com/nextcloud/server/pull/33756/files?diff=unified&w=1
Motivation
I think this is a good idea and the community seems to agree: https://redd.it/x11gue
Also Jan said that it is fine by him :)
Screenshot
click here
Signed-off-by: szaimen szaimen@e.mail.de
For my own testing