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

Rename Security and Accounts Create Button #3778

Merged
merged 7 commits into from Sep 26, 2023
Merged

Conversation

CarolineDenis
Copy link
Contributor

Fixes #3716

@CarolineDenis CarolineDenis linked an issue Jul 13, 2023 that may be closed by this pull request
Copy link
Member

@maxpatiiuk maxpatiiuk 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 to me, but let's have Grant or any other user-facing person check this

@carlosmbe carlosmbe requested a review from a team July 14, 2023 19:32
@grantfitzsimmons
Copy link
Contributor

Be warned, whatever the outcome from this, you must update this documentation:

https://discourse.specifysoftware.org/t/security-and-accounts/539

@carlosmbe as the requestor, I humbly request that you update this with the final decision once this is merged.

@grantfitzsimmons
Copy link
Contributor

I have asked @bronwyncombs to select the labels that she thinks are the most intuitive and clear for the user. I will review them once they have been implemented.

We need to be consistent throughout all areas, so we should not interchangeably use Create and Add. I will review later!

Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

I think that the buttons for User Roles are good as is for both Institution and Collections.

I think we should change the Create button below "User Accounts Assigned to this Collection" to say Assign User or something similar instead.

That would be one version of "consistent" by keeping buttons labeled Create in the Institution section. Another option would be to change both Create buttons below the User Accounts sections to Assign or Assign User. I believe I prefer the former to clearly differentiate Institution and Collection, but both options make sense.

Screen.Recording.2023-07-17.at.10.20.10.AM.mov

@carlosmbe
Copy link
Contributor

Can someone ping or slack me when this is merged so I can update the documentation.

Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

For the Add Role dialog, can the headers be different for Institution and Collection? I'd like to see the Institution Role dialog say "Add Role to Institution". If that is a complicated change, I think we should just have it read "Add Role".

Current Add Role dialog for Institution:
Screenshot 2023-07-21 at 9 58 43 AM


Everything else is great!!!

Institution Buttons
Screenshot 2023-07-21 at 10 02 27 AM

Collection Buttons
Screenshot 2023-07-21 at 10 02 53 AM

Copy link

@bronwyncombs bronwyncombs 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! 😄

Copy link
Contributor

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Looks like we could remove addRoleToCollection and then merge if it is not being used

specifyweb/frontend/js_src/lib/localization/user.ts Outdated Show resolved Hide resolved
Base automatically changed from v7.9-dev to production September 25, 2023 18:42
@CarolineDenis CarolineDenis merged commit 72107be into production Sep 26, 2023
9 checks passed
@CarolineDenis CarolineDenis deleted the issue-3716 branch September 26, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Rename Security and Accounts Create Button
5 participants