Skip to content

Cleanup styles for Geo Node Items

Zack Cuddy requested to merge cleanup-geo_node-nodes-css into master

This is part of the effort listed here: #10947 (comment 254810775)

What does this MR do?

This MR is part of an effort to replace ee/app/assets/stylesheets/pages/geo_nodes.scss with utility classes. In the process doing this will also remove the 16 lint warnings about this file. Looking through the file it is mostly colors and fixed margins. Both of which can be solved using utility classes.

The main goal of this MR is to focus on the GeoNodeItem and its respected sections. It ended up being a larger task than I had originally thought to rip out some of the legacy styles. I took my best attempt at bringing it inline with the Bootstrap Utility classes. Besides a few spacing changes, the views look identical.

Challenges/Changes worth noting:

  • Most of the tests, referenced the explicit classes. With removing them with more generic classes, it was hard to re-align the tests. So in some cases I left the original classes, however, they only serve a purpose for the tests. I wasn't sure if there was a better practice for this.

  • The actions component had a lot of un-needed divs, so I wrapped all the buttons up to just one parent.

  • There was an inconsistency between where the "Actions" lived on the GeoNodeItem. You will notice that in the Screenshot of "Healthy Node", the actions moved to the lower section. This is how the "Unhealthy Node" behaved already, so I brought those inline with each other.

  • There was one case where a @media breakpoint made the most sense. I went ahead and made that a scoped style, but if there is a better approach please let me know.

Screenshots

Node Badges Node Health Color
before Node_Badges_-_Old Node_Health_Color_-_Old
after Node_Badges_-_New Node_Health_Color_-_New
Healthy Primary Unhealthy Secondary
before Healthy_Primary_-_Old Unhealthy_Secondary_-_Old
after Healthy_Primary_-_New Unhealthy_Secondary_-_New
Sync Status Node Mobile
before Sync_Status_-_Old Node_Mobile_-_Old
after Sync_Status_-_New Node_Mobile_-_New

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Zack Cuddy

Merge request reports