-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore: Update copy on "Team Members" page to be less Editor specific #3876
Conversation
</Typography> | ||
<Typography variant="body1"> | ||
Editors have access to edit your services. | ||
Editors have access to edit your services, whilst viewers can only browse your services. |
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.
We don't need to explain the demoUser
role here as only platformAdmins
will be able to interact with this.
</AddButton> | ||
</TableCell> | ||
</TableRow> | ||
)} | ||
</Table> | ||
{showModal && ( | ||
<EditorUpsertModal | ||
<UserUpsertModal |
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.
There's still a distinction between a user
and team_members
that I think we want to maintain. This modal adds a user
record to the db, and then assigns them a role.
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.
All clear, thanks !
@@ -136,7 +136,7 @@ export const EditorUpsertModal = ({ | |||
> | |||
<Box sx={{ mt: 1, mb: 4 }}> | |||
<Typography variant="h3" component="h2" id="dialog-heading"> | |||
Add a new editor | |||
Add a new { isDemoTeam ? "demo user" : "editor" } |
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.
Should "editor"
here not be "member"
?
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.
Or is it now that we're adding a user, we can now specify the type of role they can receive?
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.
Yep exactly - this is specifically adding a user with the teamEditor
role, so "editor" is correct.
I think there's certainly scope for a role picker within this modal, but I think this is a separate task so I'm keeping thing simple here!
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.
Only grievance I would have with this is that you click "Add a team member" but receive a modal saying "Add a team editor" - feels like there's room for confusion. Could it be worth keeping the modal heading the same but alter the info text of the modal to give guidance on what permissions the team member will have?
This is a small nitty comment, so happy for it to be parked for future
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.
No, it's a very good point - we should aim for consistency here.
This will now always read "Add a new member" - this means it will be a bit more robust if we add a role picker to the modal.
e4422f9
to
3eb45c2
Compare
Removed vultr server and associated DNS entries |
What does this PR do?