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 userlist alignment #18455

Merged
merged 9 commits into from
Jan 16, 2020
Merged

fix userlist alignment #18455

merged 9 commits into from
Jan 16, 2020

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Dec 17, 2019

Fixes #18420

  • fix userlist alignment
  • add a close button for new user list
  • action buttons should be sticky

@GretaD
Copy link
Contributor Author

GretaD commented Dec 17, 2019

@skjnldsv @nextcloud/designers how we should move on with the new user list? I did a small change and now it look like this:
newlist
I am aware of the overlapping and maybe we should make the borders thicker to make it look like a sticky list over the existing list?

@gary-kim gary-kim added 2. developing Work in progress bug design Design, UI, UX, etc. labels Dec 18, 2019
@gary-kim gary-kim added this to the Nextcloud 18 milestone Dec 18, 2019
This was referenced Dec 19, 2019
@skjnldsv
Copy link
Member

I'm ok with this.
what do you think @jancborchardt

@jancborchardt
Copy link
Member

Nice @GretaD!

I am aware of the overlapping and maybe we should make the borders thicker to make it look like a sticky list over the existing list?

Can you fix that overlapping? The borders should always be only 1px, we never use thicker borders. It’s fine if it’s just sticky as it is. :)

Also, if you notice, the new entry basically duplicates all of the header info. So instead of showing it below the heading, it could be shown instead of it. (Above via z-index.)

And can you add the close button you mentioned? Next to the "Add" button on the right, just like when editing an existing user

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

See comment :)

@rullzer rullzer mentioned this pull request Jan 2, 2020
1 task
@GretaD
Copy link
Contributor Author

GretaD commented Jan 6, 2020

@jancborchardt i would say that the add new user table should be somehow visually splitted from the existing users. But yes, let me try out your proposal and see how it looks.

@GretaD
Copy link
Contributor Author

GretaD commented Jan 6, 2020

Can you fix that overlapping? The borders should always be only 1px, we never use thicker borders. It’s fine if it’s just sticky as it is. :)

what do you think? ;)
newusertable

@rullzer rullzer modified the milestones: Nextcloud 18, Nextcloud 19 Jan 9, 2020
@rullzer
Copy link
Member

rullzer commented Jan 9, 2020

master is NC19 now.
Please backport if it needs to go into 18

@GretaD GretaD force-pushed the feature/18420/new-user-alignment branch from e054640 to 97e9706 Compare January 9, 2020 16:11
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 9, 2020
@GretaD GretaD force-pushed the feature/18420/new-user-alignment branch from 97e9706 to 2556e9b Compare January 13, 2020 16:31
@jancborchardt
Copy link
Member

@GretaD is this now ready for review? :)

@GretaD GretaD added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 14, 2020
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 14, 2020
@GretaD
Copy link
Contributor Author

GretaD commented Jan 14, 2020

@m4dz i cannot add you as a reviewer but this is the full PR of what we were working :)

Copy link
Member

@georgehrke georgehrke left a comment

Choose a reason for hiding this comment

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

Looks good besides my comments

apps/settings/css/settings.scss Outdated Show resolved Hide resolved
apps/settings/css/settings.scss Outdated Show resolved Hide resolved
apps/settings/css/settings.scss Outdated Show resolved Hide resolved
apps/settings/src/components/UserList.vue Outdated Show resolved Hide resolved
apps/settings/src/components/UserList.vue Outdated Show resolved Hide resolved
@jancborchardt
Copy link
Member

jancborchardt commented Jan 15, 2020

userlist alignment

  • "Done" button should also be primary so you know what to do next
  • On editing an entry, the action buttons are too much to the right, with the 3-dot-menu being cut off half
  • How do you cancel editing a user? On editing, the 3-dot-menu should change to an X cancel icon.

@GretaD GretaD force-pushed the feature/18420/new-user-alignment branch from f8fd0a0 to f65a72f Compare January 16, 2020 13:01
@GretaD
Copy link
Contributor Author

GretaD commented Jan 16, 2020

* [ ]  "Done" button should also be primary so you know what to do next

I'll open a new ticket for this one and will work on it next week.

* [x]  On editing an entry, the action buttons are too much to the right, with the 3-dot-menu being cut off half

Done

* [ ]  How do you cancel editing a user? On editing, the 3-dot-menu should change to an X cancel icon.

I'll open a new ticket for this one and will work on it next week.

@@ -336,9 +344,9 @@ export default {
},
quotaOptions() {
// convert the preset array into objects
const quotaPreset = this.settings.quotaPreset.reduce((acc, cur) => acc.concat({
let quotaPreset = this.settings.quotaPreset.reduce((acc, cur) => acc.concat({
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let quotaPreset = this.settings.quotaPreset.reduce((acc, cur) => acc.concat({
const quotaPreset = this.settings.quotaPreset.reduce((acc, cur) => acc.concat({

Copy link
Member

Choose a reason for hiding this comment

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

Eslint fails and bundles need to be recompiled

GretaD and others added 9 commits January 16, 2020 20:12
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: GretaD <gretadoci@gmail.com>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl juliushaertl force-pushed the feature/18420/new-user-alignment branch from e835905 to a9c224c Compare January 16, 2020 19:20
@juliushaertl
Copy link
Member

Fixed eslint, css indent and pushed bundles

@rullzer rullzer merged commit c9acba1 into master Jan 16, 2020
@rullzer rullzer deleted the feature/18420/new-user-alignment branch January 16, 2020 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug design Design, UI, UX, etc. help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiline User Settings Alignment
7 participants