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

Fixes #24489: Group page UI is missing several key things #5498

Conversation

clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Mar 20, 2024

https://issues.rudder.io/issues/24489
image

  • We now have the Close button to return on home groups page (since it's diplayed in Elm we need a custom HTML event to conveniently trigger a Elm port for that)
  • The new group creation page no longer has label alignment issues (remove text-end class)
  • The groups table now appears "clickable" on every row, it always redirects to the group details
  • System groups "category" field needed a w-100 class for full width and consistency between system and non-system groups

@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@clarktsiory
Copy link
Contributor Author

PR rebased

@clarktsiory clarktsiory force-pushed the bug_24489/group_page_ui_is_missing_several_key_things branch from 318af42 to 899c206 Compare March 20, 2024 16:22
@clarktsiory clarktsiory marked this pull request as draft March 20, 2024 16:25
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

1 similar comment
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@clarktsiory clarktsiory marked this pull request as ready for review March 20, 2024 16:48
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

& "directive-save" #> SHtml.ajaxSubmit("Update", onSubmit _, ("class", "btn btn-success"))
& "directive-delete" #> deleteButton
)
} else
(
"directive-save" #> (
"directive-close" #>
Copy link
Member

Choose a reason for hiding this comment

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

I think this could have been regrouped outside of the if (it does not depend on the category to be system or not)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (after the approval but before the merge, sorry I forgot to make the change) : 9739002

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5498
-- Your faithful QA
Kant merge: "All our knowledge begins with the senses, proceeds then to the understanding, and ends with reason. There is nothing higher than reason."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/82033/console)

@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5498
-- Your faithful QA
Kant merge: "All our knowledge begins with the senses, proceeds then to the understanding, and ends with reason. There is nothing higher than reason."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/82042/console)

@clarktsiory
Copy link
Contributor Author

OK, squash merging this PR

@clarktsiory clarktsiory force-pushed the bug_24489/group_page_ui_is_missing_several_key_things branch from 9739002 to faed3d1 Compare March 22, 2024 15:10
@clarktsiory clarktsiory merged commit faed3d1 into Normation:branches/rudder/8.1 Mar 22, 2024
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants