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

chore: Update copy on "Team Members" page to be less Editor specific #3876

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Oct 30, 2024

What does this PR do?

  • Update copy on "Team Members" page to be less Editor specific, for example changing the titles from "Team Editors" to "Team Members"
  • Updates language used to code to also reflect this

@DafyddLlyr DafyddLlyr changed the title dp/team member demoUser ui chore: Update copy on "Team Members" page to be less Editor specific Oct 30, 2024
</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.
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@DafyddLlyr DafyddLlyr requested a review from a team October 30, 2024 08:33
Copy link
Member

@jessicamcinchak jessicamcinchak left a 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" }
Copy link
Contributor

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"?

Copy link
Contributor

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?

Copy link
Contributor Author

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!

Copy link
Contributor

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

Copy link
Contributor Author

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.

@DafyddLlyr DafyddLlyr force-pushed the dp/team-member-demoUser-ui branch from e4422f9 to 3eb45c2 Compare October 30, 2024 12:06
Copy link

github-actions bot commented Oct 30, 2024

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr merged commit e4900c9 into main Oct 30, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/team-member-demoUser-ui branch October 30, 2024 13:01
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.

3 participants