Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Address HANA cluster settings modal quirks #463

Merged

Conversation

dottorblaster
Copy link
Contributor

@dottorblaster dottorblaster commented Nov 16, 2021

  • the modal closes when we successfully save settings
  • now the modal closes when we click on the backdrop
  • we have a close button in the upper corner
  • pointer turns to a hand when hovering the accordion toggle
  • catalog results are now sorted and don't get mixed up every refresh

Especially:

* now the modal closes when we click on the backdrop
* we have a close button in the upper corner
* pointer turns to a hand when hovering the accordion toggle
Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

LGTM

@dottorblaster
Copy link
Contributor Author

This addresses a lot of #454

@dottorblaster dottorblaster merged commit 5a068b3 into trento-project:main Nov 16, 2021
@dottorblaster dottorblaster deleted the address-settings-modal-quirks branch November 16, 2021 16:36
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

@dottorblaster A bit late, but a change is needed

@@ -60,6 +61,11 @@ func ApiChecksCatalogHandler(s services.ChecksService) gin.HandlerFunc {
g := JSONChecksGroup{Group: group.Group, Checks: group.Checks}
groupedCatalog = append(groupedCatalog, &g)
}

sort.SliceStable(groupedCatalog, func(i, j int) bool {
Copy link
Contributor

@arbulu89 arbulu89 Nov 16, 2021

Choose a reason for hiding this comment

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

Hey @dottorblaster ,
This is not the correct way of ordering, it is not alphabetical hehe
The order of the groups is based on the name of the checks
This was already codified this way here: https://github.com/trento-project/trento/blob/main/web/models/check.go#L91

The correct fix would be to add the ordering in line 59 like this:

for _, group := range checkGroups.OrderByName() {

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of existing features
Development

Successfully merging this pull request may close these issues.

5 participants