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

Profile assignment twig migration #16360

Merged
merged 4 commits into from
Jan 22, 2024

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -

Migrates the forms to assign a profile to a user or a user to a profile to twig.
Migrates the list of profile assignments for a user to the twig datatable template.

Didn't migrate the view of users associated with a profile. I think this is the only case where we have a folder view for sub-items. Do we want to keep it, change it to a datatable, or change it in some other way?
Selection_212

Copy link
Contributor

@orthagh orthagh left a comment

Choose a reason for hiding this comment

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

Wrong pr (removed comment)

@orthagh
Copy link
Contributor

orthagh commented Jan 15, 2024

For component we may use list example from tabler : https://preview.tabler.io/lists.html (the top left one)

  • mostly .row + .col-6 for layout. Responsive in mind, we may have:
    • single col for mobile device
    • -6 cols for medium size
    • -3 cols for xxl
  • I like the fact we display user picture here, and we could enhance ui by doing the same
  • we need to find a way to display the checkbox properly on the left, it should be ok I think.

Apart this, maybe review the check all control, it seems off for the moment.

@cconard96
Copy link
Contributor Author

For component we may use list example from tabler : https://preview.tabler.io/lists.html (the top left one)

* mostly .row + .col-6 for layout. Responsive in mind, we may have:
  
  * single col for mobile device
  * -6 cols for medium size
  * -3 cols for xxl

* I like the fact we display user picture here, and we could enhance ui by doing the same

* we need to find a way to display the checkbox properly on the left, it should be ok I think.

So, keep the folder view but replace the display of the user names with a list like in the tabler demo?

@orthagh
Copy link
Contributor

orthagh commented Jan 15, 2024

yes exactly

@cconard96 cconard96 requested a review from orthagh January 18, 2024 15:11
@orthagh
Copy link
Contributor

orthagh commented Jan 22, 2024

last thing bothering me for this pr:

  • the user's avatar on a different column than username.
    This leads to an annoying spacing between the both.
  • if a filter return no data, there is no way to reset (apart refreshing the page). Maybe it's generalized on all page using filters.

Apart these, it's ok for me

@cconard96 cconard96 force-pushed the ui/profile_auth_twig branch from bd19f1d to 1da0cfd Compare January 22, 2024 14:44
@cconard96 cconard96 force-pushed the ui/profile_auth_twig branch from 1da0cfd to 9935c82 Compare January 22, 2024 14:45
src/Profile_User.php Outdated Show resolved Hide resolved
src/Profile_User.php Outdated Show resolved Hide resolved
templates/pages/admin/add_profile_authorization.html.twig Outdated Show resolved Hide resolved
src/Profile_User.php Outdated Show resolved Hide resolved
src/Profile_User.php Outdated Show resolved Hide resolved
src/Profile_User.php Outdated Show resolved Hide resolved
@cedric-anne cedric-anne added this to the 10.1.0 milestone Jan 22, 2024
@cedric-anne cedric-anne merged commit 1f858cb into glpi-project:main Jan 22, 2024
8 checks passed
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