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 role map for Show Toolbar Permission #2108

Closed
wants to merge 1 commit into from
Closed

Conversation

fgrcon
Copy link
Member

@fgrcon fgrcon commented Jul 20, 2017

see end of discussion of #1570

Copy link
Member

@agitator agitator left a comment

Choose a reason for hiding this comment

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

idea was to not change the default behavior, but if others are fine with it, i'm too

Copy link
Member

@hvelarde hvelarde left a comment

Choose a reason for hiding this comment

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

@fgrcon I'm totally against this change: the toolbar must be fixed as a whole and not just to solve your specific use case.

as mentioned before, I agree with @jensens: all authenticated users must see at least a toolbar with the personal bar or we'll be creating new basic usability problems like user doesn't know she is logged-in and user doesn't know how to log out.

if you want to hide the toolbar on your projects, do that as part of your policy.

@jensens
Copy link
Sponsor Member

jensens commented Jul 20, 2017

yes, please add Authenticated to the defaults - if you then remove it in your policy thats fine.

@fgrcon
Copy link
Member Author

fgrcon commented Jul 20, 2017

absolutely sure about that. ;-)

@fgrcon
Copy link
Member Author

fgrcon commented Jul 20, 2017

Bye

1 similar comment
@fgrcon
Copy link
Member Author

fgrcon commented Jul 20, 2017

Bye

@fgrcon
Copy link
Member Author

fgrcon commented Jul 21, 2017

after a while (but still a little bit annoyed ...)
@hvelarde: I really would appreciate that you start reading stuff more carefully before commenting. This is a regression in ux/ui. Untill Plone 4.x an authenticated user only saw a toolbar (edit bar) when he had the appropriate locale roles.

@jensens: adding authenticated renders all of @agitator s changes useless. But if you prefer to force plone users to create policies for something which really should be default behavior - well ok for me ..

@fgrcon fgrcon closed this Jul 21, 2017
@agitator
Copy link
Member

@fgrcon you are right about the "regression"

uiwise there is the problem that the personal actions would switch between top-right and bottom left depending on the users permission, which would be inconsistent.

in my usecase, an intranet, i'm adding additional edit-tools within the content area and never show the toolbar for that role, so that the user doesn't have that inconsistency.

@hvelarde
Copy link
Member

hvelarde commented Jul 21, 2017

@fgrcon of course we have a regression and many other problems related to the toolbar: previously we had different elements: an edit bar and a personal bar among them; today we have only one big toolbar with huge UI/UX problems and I've being pointing to them even before the release of Plone 5 as you can see in #957.

you are messing around the Show Toolbar permissions, not just the edit part of it; you were about to create even more problems, because you're not thinking in the whole picture but only in solving your own problem.

so start by being careful yourself and be open to criticism: @jensens had already rose the concern before me and you just ignored both of us.

@hvelarde hvelarde deleted the fgrcon-fix-1570 branch July 21, 2017 12:31
@jensens
Copy link
Sponsor Member

jensens commented Jul 21, 2017

The problem of a more consistent editing UI is bigger than a bunch of small fixes introducing new problems. I hope Pastanaga will help in future. We may also improve the current toolbar, but this needs to be done carefully with discussions, probably best at a sprint or conference with a wide variety of Plonistas attending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants