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

Panel\Field::role() allow passing $roles #6654

Draft
wants to merge 7 commits into
base: develop-minor
Choose a base branch
from

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Sep 7, 2024

Description

Summary of changes

  • Allow passing roles collection to Panel\Field::role()
  • Use roles collection methods for logic instead foreach
  • Only hide field if no option available. Otherwise show as radio button for transparency which role is chosen.
  • User create dialog preselects first role option instead of the role of the current user (or is the assumption users create mostly other users with the same role as theirs?)

Reasoning

Different action contexts will have a different set of roles available to display. Instead of putting that logic into this UI method, it should rather allow to pass the appropriate roles collection to this helper method.

Additional context

We will modify the actual roles collection passed in the dialogs in a subsequent PR. We keep them basic for now to have no change in behavior.

Changelog

Fixes

Enhancements

  • Role always shown when creating a new user, even if only one role available

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

@distantnative distantnative added the type: enhancement ✨ Suggests an enhancement; improves Kirby label Sep 7, 2024
@distantnative distantnative added this to the 4.5.0 milestone Sep 7, 2024
@distantnative distantnative self-assigned this Sep 7, 2024
@distantnative distantnative force-pushed the enhancement/panel-field-role branch 2 times, most recently from ee6c42b to 54481de Compare September 7, 2024 20:46
@distantnative distantnative marked this pull request as ready for review September 7, 2024 20:58
@distantnative distantnative requested a review from a team September 7, 2024 20:58
@distantnative distantnative marked this pull request as draft September 8, 2024 08:01
@distantnative distantnative removed the request for review from a team September 8, 2024 08:01
Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure why, but when I test this PR in the sandbox lab, I can only add users with admin roles. The other roles vanish from the field.

@distantnative
Copy link
Member Author

@bastianallgeier It's because of this https://github.com/getkirby/kirby/blob/develop-minor/src/Cms/User.php#L588-L591 - for the last admin we exit here too early. That's fine for changing the role, but not creating. Maybe we need to first merge #6657 and then can remove that check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants