-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
Users & permissions - remaining frontend work #2524
Conversation
… into permissions-access-modals
…permissions-access-modals
…permissions-access-modals
…permissions-access-modals
…/explorations/${queryId}/edit`
…permissions-access-modals
@kgodey I require your review on the UX and the behaviour as mentioned in the description. The UX is not good enough for my own expectations, but I don't see a better solution at the moment with the API constraints we currently have. |
The frontend implementation of permission levels should be consistent with the backend. However, since this is erring on the side of decreased permissions, I'm fine with treating it as a non-launch blocking bug. Please open an issue to track fixing it.
We can fix this after launch if it will take time to implement, but here are the changes I'd make:
|
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.
See comment in main thread.
I agree. I'll open an issue to track it, however, I would like to discuss the behaviour of this at the product level. My suggestion is to go with the behaviour implemented on the frontend as of now, or come up with a better UX to let the user understand which of the roles take precedence. We can discuss this on the new issue.
This will not take a lot of time, I can make these changes today. Does the following UX look good? (I've not made these changes yet). Notice that the second row appears disabled, inorder to indicate to the user that these permissions don't take any effect as they are superseded by the schema level permissions. I can add a tooltip when user hovers that row. |
Let's discuss this on the new issue. I think we probably should come up with a better UX.
Yes, looks good. |
Fixes #2473
Fixes #2474
Fixes #2478
Fixes #2479
Fixes #2480
The following differs from the spec:
viewer
on db, andmanager
on schema, the user will havemanager
access on the schema.manager
on db, andeditor
on schema, the user will havemanager
access on the schema.viewer
on db, andmanager
on schema, the user will havemanager
access on the schema.manager
on db, andeditor
on schema, the user will haveeditor
access on the schema.Checklist
Update index.md
).master
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin