Auto-add permissions/unsaved permissions prompt

starter-task
pr-welcome

(Mark Walkom) #1

When editing permissions, it can be easy to add a new set but forget to click the Add Permission to apply it before closing the edit box (don’t judge me! :p).

Would it be possible to have either a reminder - saying Unapplied Permissions or similar - or just apply the new permissions when clicking Save Category?


When creating new category with "staff" permission, it is only setting the permission to "everyone"
(Rafael dos Santos Silva) #2

I second this. I did forget the apply too many times!


(Simon Cossar) #3

I have forgotten to to this as well, but making a mistake by adding the wrong group to category permissions could have serious consequences. I think having requiring confirmation here is a good idea.

Without a confirmation, there would need to be some way to dismiss a permission after the Edit Permissions button has been clicked.

44%20PM

If it needs to be changed, maybe the Add Permission button could be changed to a green checkmark box, similar to what is used in the Setting controls.


(Jeff Atwood) #4

Maybe a good starter task @erlend_sh?


#5

Hello,

I wanted to contribute and looked at this issue, I think it would be good enough if the design could be changed: By renaming the “Add Permission” Button into just “Add” and moving it to the same line as the combo-boxes:


It would be intuitive to click, rather than changing the logic (which would involve checking if something has changed in the combo-boxes and then showing a Message).
Let me know what you think


#6

Hello, I made the change and created a PR:
https://github.com/discourse/discourse/pull/6999


(Sam Saffron) #7

I worry that this will not fit in some non English locales. maybe if its in some magic flexbox it would work. @awesomerobot can you have a very quick look at this.


#8

Yes, I figured I change it to a “+” instead of “Add”, sorry I haven’t mentioned that in the previous post


(Sam Saffron) #9

I can see:

    margin-left: 0.25em;
    position: absolute;

so there is still a real chance this thing will overlap, we should probably use a flexbox here or float the element right or something.


(Kris) #11

Commented inline on Github, but that’s the gist — position: absolute is unneeded here. Everything else seems ok with long strings.

Good call, removing text certainly makes localization easier.


#12

Thanks a lot for the review, I changed it accordingly and added a right margin to the permission-selector.
In order to make the add-button align with the combo-boxes, I did the following:
.add-permission { position: relative; top: 0.1em; }
otherwise the button would float a few pixels higher then the combo-boxes.
Open for suggestions if there’s a better way to do it :slight_smile:


(Kris) #13

Great, I just merged in the change UX: make adding permissions more visible and intuitive to click. (#6999) · discourse/discourse@159dd69 · GitHub. The button alignment is fine for now, there’s a bigger fix we’ll work on that should better resolve the alignment issues.