EDSC-4322: Refactor `CustomizableIcons.jsx` and fix styling inconsistencies #1842

eudoroolivares2016 · 2024-12-04T16:39:51Z

Overview

What is the feature?

Standardize the classanames for cusomizableIcons.jsx, removes using actual portals on the unit test so that it won't fail if we make updates to portal configurations, fixes an issue in production where long harmony names are overlapping with text on the icons, some colors on the granule download options were not visible this updates their color so that they can more easily be seen

What is the Solution?

Updates the cusomizableIcons.jsx component to use the established pattern of having its own scss file to enforce lose coupling and making sure style changes to one component do not affect another one.

Updating the colors for a few buttons on the granule download links using carbon-50 to match icons we have on the order-status page

What areas of the application does this impact?

List impacted areas.

Testing

Reproduction steps

**Environment for testing:Prod
**Collection to test with:See below

Collection with long Harmony service name C1996881146-POCLOUD
Collection that has granules with S3 and download files: C1214470488-ASF
Collections in general filter using the customizable facet to see ensure the icons are appearing as expected

Attachments

Current ops:

Feature Branch:

Please include relevant screenshots or files that would be helpful in reviewing and verifying this change.

Checklist

I have added automated tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings

codecov · 2024-12-04T16:43:52Z

Codecov Report

Attention: Patch coverage is 91.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 93.78%. Comparing base (fcae98f) to head (535e25f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...tionsIcons/AvailableCustomizationsTooltipIcons.jsx	78.26%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1842      +/-   ##
==========================================
- Coverage   93.81%   93.78%   -0.04%     
==========================================
  Files         777      778       +1     
  Lines       18867    18885      +18     
  Branches     4867     4847      -20     
==========================================
+ Hits        17701    17711      +10     
- Misses       1090     1095       +5     
- Partials       76       79       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

eudoroolivares2016 · 2024-12-04T16:47:10Z

static/src/js/components/Tour/TourSteps.jsx

@@ -642,7 +642,7 @@ const TourSteps = ({
              <ExternalLink
                href="https://www.earthdata.nasa.gov/learn/earthdata-search"
              >
-                Earthdata Search wiki
+                Earthdata Search


Some discussion with PO around this; since this link is not actually a wiki page this language change is preferred

eudoroolivares2016 · 2024-12-04T16:47:35Z

static/src/js/components/SearchSidebar/__tests__/SearchSidebarHeader.test.js

@@ -25,8 +25,7 @@ import SearchSidebarHeader from '../SearchSidebarHeader'
 import SearchFormContainer from '../../../containers/SearchFormContainer/SearchFormContainer'
 import PortalLinkContainer from '../../../containers/PortalLinkContainer/PortalLinkContainer'

-// eslint-disable-next-line import/no-unresolved
-import availablePortals from '../../../../../../portals/availablePortals.json'
+import availablePortals from './availablePortalsMock.json'


We have a tiny ticket to not use real portal configs in the unit tests

README.md

eudoroolivares2016 · 2024-12-04T18:39:31Z

static/src/js/components/GranuleResults/GranuleResultsDataLinksButton.scss

@@ -9,7 +9,7 @@
    cursor: pointer;

    .button__icon {
-      color: rgba($color__spacesuit-white, 0.7);
+      color: $color__carbon--40;


this should maybe be $color__carbon--50; since that is what we are using in static/src/js/components/CopyableText/CopyableText.scss

I made it the same color as the nearby copy button so they all match this button I mean:

EDSC-4322: revert style change EDSC-4322: Match colors

trevorlang

Still working through the last bits of the PR review but I have a few tweaks in mind that will help with the design a bit. It would be great if we could get them in now while were touching this.

The line height on the Harmony services on the project page is too tight. We should remove the line-height: 1 declaration on .harmony-select-item. This is also affecting the customizable icons and makes those look better
When an access method hovered, the background of the customizable icons is too similar to the background of the access method. We should add some css to the .access-method so that when one is hovered, the customizable icon badge uses one step darker versions of the gray than the default.
The background color change during the hover state on the customizable icons and other badges is not serving any purpose and might make the icons look clickable, so I want to get rid of that. To fix that, we should:
- Remove the hover state css on .customizable-icons__meta-icon
- Remove the hover state on css`.collection-results-item__meta-icon
On harmony services on the project page, the customizable icons are not aligned with the service title. Adding align-items: flex-start on the .access-method-radio__header-primary class will clean that up.

EDSC-4322: remove unused class

eudoroolivares2016 · 2024-12-14T01:49:37Z

static/src/js/components/AvailableCustomizationsIcons/AvailableCustomizationsTooltipIcons.jsx

+  hasCombine: false
+}
+
+availableCustomizationsTooltipIcons.propTypes = {


I tried combining these two the issue becomes that these are props to MetaIcon when its used so we'd need to refactor MetaIcon as well and its being used that way in other places as well for non customizable icons

bnp26

@eudoroolivares2016 Personally I find the "Available" part of the new components to be a bit redundant and like the old name better but that's a matter of preference, then the new tooltips component would be named CustomizableTooltipIcons.jsx

If you do decide to move forward with the new naming, it might be better english to call the new customization icons to be
AvailableCustomizationIcons.jsx instead of AvailableCustomizationsIcons.jsx but that's up to you.
Similarly, the Icons would be AvailableCustomizationTooltipIcons.jsx instead of AvailableCustomizationsTooltipIcons.

static/src/js/components/AvailableCustomizationsIcons/AvailableCustomizationsIcons.jsx

static/src/js/components/SearchSidebar/__tests__/availablePortalsMock.json

trevorlang

This looks good to me other than some tiny tweaks to colors to improve contrast on the hover states. I think we should improve that while we're in here.

trevorlang · 2024-12-18T21:57:42Z

static/src/js/components/AccessMethod/AccessMethod.scss

Not related to your PR, but could you tweak the hover states on lines 58 and 101 to be $color__carbon--5, instead of $color__carbon--10? That extra contrast will be good.

…er selected buttons when hovered

bnp26 · 2024-12-19T16:35:14Z

@eudoroolivares2016 If you're gonna change the component name to AvailableCustomizationIcons, you should also change the filenames to say those as well

eudoroolivares2016 commented Dec 4, 2024

View reviewed changes

eudoroolivares2016 force-pushed the EDSC-4322 branch from d54e706 to deec39a Compare December 4, 2024 17:09

eudoroolivares2016 commented Dec 4, 2024

View reviewed changes

eudoroolivares2016 force-pushed the EDSC-4322 branch from d73af0e to 3722ef7 Compare December 5, 2024 15:03

eudoroolivares2016 marked this pull request as ready for review December 5, 2024 16:08

eudoroolivares2016 added 2 commits December 6, 2024 13:43

EDSC-4322: Fix portal mock test

14284e9

EDSC-4322: Fix Customizable Icons styling

13cdb9d

eudoroolivares2016 force-pushed the EDSC-4322 branch from 3722ef7 to 6cc8f92 Compare December 6, 2024 18:45

eudoroolivares2016 requested review from macrouch, trevorlang, dpesall, daniel-zamora and dmistry1 December 6, 2024 19:29

EDSC-4322: Match colors of nearby similar buttons

c96ede4

EDSC-4322: revert style change EDSC-4322: Match colors

eudoroolivares2016 force-pushed the EDSC-4322 branch from 6cc8f92 to c96ede4 Compare December 9, 2024 16:45

dpesall approved these changes Dec 11, 2024

View reviewed changes

trevorlang reviewed Dec 12, 2024

View reviewed changes

eudoroolivares2016 added 8 commits December 13, 2024 14:07

EDSC-4322: Refactor customizable icons and where its being called

a02edaf

EDSC-4322: Fixing customizable icon tooltips

b4d2ca1

EDSC-4322: Clean up styling remove unused classes

c813288

EDSC-4322: remove unused class

EDSC-4322: Renaming files

0ce75ef

EDSC-4322: rename var

2c66bc9

EDSC-4322: Add tests and fix one test mistake

73f3785

EDSC-4322: Fix readme mistake

8538ec4

ESDC-4322: Cleanup

3d40d3f

eudoroolivares2016 requested a review from trevorlang December 14, 2024 01:52

eudoroolivares2016 commented Dec 16, 2024

View reviewed changes

EDSC-4322: Update var name remove class not being used

5f4e59b

eudoroolivares2016 changed the title ~~EDSC-4322: Cleanup the classnames in CustomizableIcons.jsx~~ EDSC-4322: Refactor CustomizableIcons.jsx and remove reused CSS Dec 16, 2024

eudoroolivares2016 changed the title ~~EDSC-4322: Refactor CustomizableIcons.jsx and remove reused CSS~~ EDSC-4322: Refactor CustomizableIcons.jsx and fix styling inconsistencies Dec 16, 2024

bnp26 requested changes Dec 18, 2024

View reviewed changes

trevorlang requested changes Dec 18, 2024

View reviewed changes

EDSC-4322: Fix naming convention, tweak background color to match oth…

2258a52

…er selected buttons when hovered

eudoroolivares2016 requested review from bnp26, trevorlang and dpesall December 19, 2024 14:56

EDSC-4322: Pluralize consistently

535e25f

bnp26 approved these changes Dec 20, 2024

View reviewed changes

dpesall approved these changes Dec 20, 2024

View reviewed changes

trevorlang approved these changes Dec 27, 2024

View reviewed changes

eudoroolivares2016 merged commit 3f1a79a into main Dec 30, 2024
11 checks passed

eudoroolivares2016 deleted the EDSC-4322 branch December 30, 2024 19:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EDSC-4322: Refactor `CustomizableIcons.jsx` and fix styling inconsistencies #1842

EDSC-4322: Refactor `CustomizableIcons.jsx` and fix styling inconsistencies #1842

eudoroolivares2016 commented Dec 4, 2024 •

edited

Loading

codecov bot commented Dec 4, 2024 •

edited

Loading

eudoroolivares2016 Dec 4, 2024

eudoroolivares2016 Dec 4, 2024 •

edited

Loading

eudoroolivares2016 Dec 4, 2024

eudoroolivares2016 Dec 5, 2024

trevorlang left a comment •

edited

Loading

eudoroolivares2016 Dec 14, 2024

bnp26 left a comment

trevorlang left a comment

trevorlang Dec 18, 2024

bnp26 commented Dec 19, 2024

EDSC-4322: Refactor CustomizableIcons.jsx and fix styling inconsistencies #1842

EDSC-4322: Refactor CustomizableIcons.jsx and fix styling inconsistencies #1842

Conversation

eudoroolivares2016 commented Dec 4, 2024 • edited Loading

Overview

What is the feature?

What is the Solution?

What areas of the application does this impact?

Testing

Reproduction steps

Attachments

Checklist

codecov bot commented Dec 4, 2024 • edited Loading

Codecov Report

eudoroolivares2016 Dec 4, 2024

Choose a reason for hiding this comment

eudoroolivares2016 Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

eudoroolivares2016 Dec 4, 2024

Choose a reason for hiding this comment

eudoroolivares2016 Dec 5, 2024

Choose a reason for hiding this comment

trevorlang left a comment • edited Loading

Choose a reason for hiding this comment

eudoroolivares2016 Dec 14, 2024

Choose a reason for hiding this comment

bnp26 left a comment

Choose a reason for hiding this comment

trevorlang left a comment

Choose a reason for hiding this comment

trevorlang Dec 18, 2024

Choose a reason for hiding this comment

bnp26 commented Dec 19, 2024

EDSC-4322: Refactor `CustomizableIcons.jsx` and fix styling inconsistencies #1842

EDSC-4322: Refactor `CustomizableIcons.jsx` and fix styling inconsistencies #1842

eudoroolivares2016 commented Dec 4, 2024 •

edited

Loading

codecov bot commented Dec 4, 2024 •

edited

Loading

eudoroolivares2016 Dec 4, 2024 •

edited

Loading

trevorlang left a comment •

edited

Loading