Skip to content

Conversation

@ciyer
Copy link
Contributor

@ciyer ciyer commented Nov 17, 2025

Summary

Use a react-select component to set the role for group members. This allows us to show more detailed information about the differences between the roles.

image

/deploy

@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3906.dev.renku.ch

@ciyer ciyer marked this pull request as ready for review November 17, 2025 09:20
@ciyer ciyer requested a review from a team as a code owner November 17, 2025 09:20
@ciyer ciyer changed the title feat: use react-selct to pick group member role feat: use react-select to pick group member role Nov 17, 2025
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Nice addition 🚀

Minor change: I suggest we use the standard styling/positioning for dropdown menus, with the label above the input. It looks a little off since there is already another dropdown in the same modal (on the left is the proposal, on the right the current implementation)

Image

@lorenzo-cavazzi lorenzo-cavazzi self-assigned this Nov 18, 2025
@ciyer ciyer force-pushed the ciyer/explain-group-roles branch from da45455 to 5c8374c Compare November 18, 2025 10:00
@ciyer
Copy link
Contributor Author

ciyer commented Nov 18, 2025

Minor change: I suggest we use the standard styling/positioning for dropdown menus, with the label above the input.]

I agree, it looks better when using the our standard.

image

Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Lgtm! 🚀

I wonder if we should plan to have a similar "select" for project permissions -- perhaps it's less confusing in that case, but the difference between Editor and Owner might not be obvious.
I'll make an issue

Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

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

Tiny detail I initially missed

fieldState: { error },
}) => (
<>
<div className={cx("ms-1", "w-100", error && "is-invalid")}>
Copy link
Member

Choose a reason for hiding this comment

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

I noticed late this misalignement

Image
Suggested change
<div className={cx("ms-1", "w-100", error && "is-invalid")}>
<div className={cx("w-100", error && "is-invalid")}>

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