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

Don't show 'Personal' header in settings when Admin section is not there #14666

Merged
merged 2 commits into from
Mar 18, 2019
Merged

Don't show 'Personal' header in settings when Admin section is not there #14666

merged 2 commits into from
Mar 18, 2019

Conversation

jancborchardt
Copy link
Member

@jancborchardt jancborchardt commented Mar 13, 2019 •
edited
Loading

For regular users we don’t need to show the "Personal" heading because there’s no other headings anyway. For admins nothing changes.
Personal settings before settings after

@jancborchardt jancborchardt added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews feature: settings labels Mar 13, 2019
@jancborchardt jancborchardt added this to the Nextcloud 17 milestone Mar 13, 2019
@jancborchardt jancborchardt mentioned this pull request Mar 13, 2019
Closed
rullzer
rullzer approved these changes Mar 13, 2019
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

sure

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 15, 2019
@MorrisJobke
Copy link
Member

@jancborchardt Mind to rebase on current master, because there were quite some fixes merged lately.

@rullzer rullzer force-pushed the design/settings-header branch from f8a604c to a25f956 Compare March 15, 2019 15:36
@rullzer
Copy link
Member

rullzer commented Mar 15, 2019

This breaks the acceptance tests that checks for the personal header :P

@rullzer rullzer added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Mar 16, 2019
@jancborchardt
Don't show 'Personal' header in settings when Admin section is not there
33bafb8 
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt jancborchardt force-pushed the design/settings-header branch from a25f956 to 33bafb8 Compare March 18, 2019 12:14
@jancborchardt
Copy link
Member Author

Rebased on master, but the test is not so easy to fix, as if we would change the:

Then I see that the "Personal" settings panel is shown

it wouldn’t test for the entire section? So it should check for the content instead of the heading. (FYI I’m on vacation and it’s out of my abilities, so I’m happy for someone to take over. :)

@MorrisJobke MorrisJobke self-assigned this Mar 18, 2019
@MorrisJobke
Adjust acceptance tests
970d52f 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

it wouldn’t test for the entire section? So it should check for the content instead of the heading. (FYI I’m on vacation and it’s out of my abilities, so I’m happy for someone to take over. :)

I adjusted and fixed them 👍

MorrisJobke
MorrisJobke approved these changes Mar 18, 2019
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.

Looks nice 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Mar 18, 2019
@MorrisJobke
Copy link
Member

Let's get this into 16 as it is just a small fix

@faily-bot
Copy link

faily-bot bot commented Mar 18, 2019

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 17117: failure

TESTS=carddavtester-old-endpoint

@MorrisJobke
Copy link
Member

Completely unrelated failure. I haven't seen this before.

@MorrisJobke MorrisJobke merged commit 4824d27 into master Mar 18, 2019
@MorrisJobke MorrisJobke deleted the design/settings-header branch March 18, 2019 13:48
@MorrisJobke MorrisJobke mentioned this pull request Mar 20, 2019
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@rullzer rullzer rullzer approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

@MorrisJobke MorrisJobke

Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement feature: settings
Projects
None yet
Milestone
Nextcloud 16
Development

Successfully merging this pull request may close these issues.
5 participants
@jancborchardt @MorrisJobke @rullzer @juliusknorr @ChristophWurst