@diosmosis opened this Pull Request on January 13th 2021 Member

Description:

Contains the following fixes:

  • the logic used to enable the bulk actions dropdown based on selected row count was not behaving correctly, since the model of checkboxes was being updated after the ng-click fired. fixed w/ a $timeout.
  • fix overlapping selects using an event available in materializecss. selects would overlap because they had the same z-index as each other. we used to try and change the z-indexes on focus and on blur, but blur is invoked right after focus. the materializecss event, however, is fired at the right time.
  • since one user has trouble viewing a certain unicode character, switched the 2FA column in the users management table to use an icon. might be confusing since the icon is also used for actions, elsewhere.

Fixes #17079

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@sgiehl commented on January 13th 2021 Member

The role dropdown still looks broken in chrome:
image

This seems to be caused by the transform: scale(0.8) on the select-wrapper. Disabling it fixes the issue. Guess the reason it that transform creates a new stacking context for z-index

@diosmosis commented on January 13th 2021 Member

@sgiehl odd, it was working for me. I'll look again.

@diosmosis commented on January 14th 2021 Member

@sgiehl i'm not able to reproduce:

imageimage

actually... was just able to reproduce. it only happens if you click on another select while one is open. Will fix by removing transform, much simple solution.

@sgiehl commented on January 14th 2021 Member

That fix works for me on chrome. But shouldn't we make the font-size in the drop down smaller as well. Guess that was the case before (due to the transform of the parent element)

Also I just found another issue in the user management. The role drop down is also showing the capabilities. Is that on purpose? Selecting one of the capabilities even doesn't add it for the user.

@diosmosis commented on January 17th 2021 Member

That fix works for me on chrome. But shouldn't we make the font-size in the drop down smaller as well. Guess that was the case before (due to the transform of the parent element)

I'm not sure it's really needed. The transform was just so it didn't stretch out the rows in the table. Personally I don't think it really matters the dropdown font is different. I can change it if needed, though.

@sgiehl commented on January 18th 2021 Member

@diosmosis we can also keep it that way. But imho the drop down content looks a bit oversized, compared to the other elements.

image

@diosmosis commented on January 20th 2021 Member

@sgiehl updated the dropdown text height:

image
@sgiehl commented on January 20th 2021 Member

@diosmosis looks better now. Would be good to merge from my side. But there are a couple of UI tests failing due to this changes...

This Pull Request was closed on January 22nd 2021
Powered by GitHub Issue Mirror