EDSC-4252: Update the Application header to match the design in post tophat2 update #1816

eudoroolivares2016 · 2024-10-24T18:46:37Z

Overview

What is the feature?

Integrating tophat2's new features onto earthdata search to deprecate the additional header. Implementing design to move secondary toolbar buttons as an overlay on each applicable page

What is the Solution?

Removing the Application header creating floating buttons for the secondary toolbar onto the map

What areas of the application does this impact?

Changes the app header, locations of colormap and legend, updates the secondary toolbar that appears on most application pages

Testing

Reproduction steps

**Environment for testing:ANY
**Collection to test with:NA

Ensure that all pages of EDSC have access to the top level header
Regression test to ensure that creating projects, using the tour, logging in, all work as expected
Ensure that visually the components look like the original design specifications
Ensure that colormaps are placed further down near the leaflet controls
Ensure that the tophat2 like works as expected to bring you back to the top level page from anywhere in the application

Attachments

When Error Banner is up

When Colormap is up

Toplevel page:

Preferences but, looks the same on all the pages

Order Status page:

Project page:

My project saved project

Not found page:

Logged out:

These buttons do in fact line-up

The order status page:

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-10-24T18:50:58Z

Codecov Report

Attention: Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.02%. Comparing base (ecf5fdb) to head (edf8de1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
static/src/js/App.jsx	50.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1816      +/-   ##
==========================================
- Coverage   93.46%   91.02%   -2.44%     
==========================================
  Files         772      770       -2     
  Lines       18555    18565      +10     
  Branches     4784     4788       +4     
==========================================
- Hits        17342    16899     -443     
- Misses       1165     1524     +359     
- Partials       48      142      +94

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

eudoroolivares2016 · 2024-10-25T16:43:54Z

static/src/js/containers/MapContainer/MapContainer.scss

@@ -21,4 +17,14 @@ $headerHeight: 2rem;
  .is-panels-dragging & {
    pointer-events: none;
  }
+
+  &__color-map {


This might be OBE

eudoroolivares2016 · 2024-10-25T18:00:34Z

static/src/js/components/Panels/Panels.jsx

-      // map tools.
-      return routeWrapperWidth - 55
+      // Set the maxWidth to the available space minus the width of the secondary toolbar with some buffer.
+      const secondaryToolbarWidth = document.querySelector('.secondary-toolbar').offsetWidth


This is getting tested by playwright panels.spec.js

static/src/js/components/SecondaryToolbar/SecondaryToolbar.jsx

static/src/js/components/SecondaryToolbar/SecondaryToolbar.scss

… for borders around the ma

…acement, and scale-control placement - Revert banner to HEAD of main

…efault test

…that fails on GH actions

…aving issues

eudoroolivares2016 · 2024-10-29T18:40:25Z

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

@@ -103,6 +103,7 @@ const SearchTour = () => {
      stepIndex={stepIndex}
      continuous
      callback={handleJoyrideCallback}
+      disableScrollParentFix


https://docs.react-joyride.com/props - this was to fix an issue for some cases on joyride in our case with the wrapping-container this was adding a style that messed up the overflows we added in SCSS
it was adding overflow: initial to the parent element of the Search Tour

…ger needs logic, fix state for initial project value

eudoroolivares2016 · 2024-10-29T19:56:14Z

static/src/js/components/Errors/NotFound.jsx

@@ -22,7 +22,7 @@ export const NotFound = ({

  // Modify the background color of root element for the not found page so we can load stars jpg
  const selectElementById = () => {
-    const element = document.getElementById('app')
+    const element = document.getElementById('wrapping-container')


This is needed because we can't set this to black with the map selector unless we add a list of all the routes etc. Since this is the route we will be going to if it matches none of our routes

This is interesting. Not a deal breaker but Id be curious to see if react-router had a way to check to see if any routes were matched on the current page.

This reverts commit cf03ea6.

…spot

…aywright tests

trevorlang · 2024-10-30T15:43:44Z

static/src/js/components/Errors/NotFound.jsx

@@ -22,7 +22,7 @@ export const NotFound = ({

  // Modify the background color of root element for the not found page so we can load stars jpg
  const selectElementById = () => {
-    const element = document.getElementById('app')
+    const element = document.getElementById('wrapping-container')


This is interesting. Not a deal breaker but Id be curious to see if react-router had a way to check to see if any routes were matched on the current page.

trevorlang · 2024-10-30T15:45:38Z

static/src/js/components/Panels/Panels.jsx

-      // Set the maxWidth to the available space minus the width of the
-      // map tools.
-      return routeWrapperWidth - 55
+      return routeWrapperWidth - 165


Maybe add back a comment here so we know why were subtracting. Something simple like this would probably work.

Subtracting from the available space to ensure the user menu/login button and map tools remain visible at the maximum panel width

trevorlang · 2024-10-30T15:47:47Z

static/src/js/components/SecondaryToolbar/SecondaryToolbar.jsx

@@ -87,6 +89,7 @@ class SecondaryToolbar extends Component {
    onUpdateProjectName(newProjectName)
  }

+  // Needed for Save Project so page does not change


I think you could elaborate here. Not sure exactly what you mean by this comment. Ideally were using full sentences and adding all the required details to understand the implementation more easily.

Updated. we discussed this at one point as a team if we needed some of the event.stopPropagation() calls but, we do because if we don't have that when the project-id updates the url path it tries to refresh the page. (If you remove them I mean)

static/src/js/components/SecondaryToolbar/SecondaryToolbar.jsx

static/src/js/components/SecondaryToolbar/SecondaryToolbar.scss

trevorlang · 2024-10-30T15:57:04Z

static/src/js/containers/MapContainer/MapWrapper.jsx

The spacing here looks a little wonky when a colormap is visible. If possible, it would be great if there was the same about of space between the map tools and colormap as there is between the colormap and scale

The only issue with that is that is really going to break a significant amount of the map.spec tests

The inconsistent spacing looks pretty bad. If we cant get it in now, we should write an improvement ticket to track it.

I've added those to the https://bugs.earthdata.nasa.gov/browse/EDSC-4302 ticket as new AC with the comment in the description

static/src/js/containers/WrappingContainer/WrappingContainer.jsx

trevorlang

One last tweak 😄

trevorlang · 2024-10-30T17:33:04Z

static/src/js/containers/WrappingContainer/WrappingContainer.jsx

@@ -15,12 +15,14 @@ export const WrappingContainer = (props) => {
  const { search, pathname } = location
  let isMapPage = ['/search']

-  // Currently saved projects and a project page share route1
+  // Currently saved projects and a project page share route as such we must determine if we are on the saved projects page


Currently saved projects and a project page share a route as such we must determine if we are on the saved projects page

😄

that's what I get for going fast 🤦

…in subsequent ticket

eudoroolivares2016 force-pushed the EDSC-4252 branch from 89cfca7 to 579299b Compare October 24, 2024 18:47

eudoroolivares2016 changed the title ~~Edsc 4252~~ EDSC-4252 Oct 24, 2024

eudoroolivares2016 changed the title ~~EDSC-4252~~ EDSC-4252: Update the Application header to match the design in post tophat2 update Oct 25, 2024

eudoroolivares2016 force-pushed the EDSC-4252 branch from 919464f to dfa6f01 Compare October 25, 2024 16:42

eudoroolivares2016 commented Oct 25, 2024

View reviewed changes

eudoroolivares2016 added 20 commits October 25, 2024 14:52

EDSC-4252: First working commit progress

62dff0a

EDSC-4252: Adjust the size of the panel width; conditional classnames…

c3d5279

… for borders around the ma

EDSC-4252: Fix icons and some of the look

7a737cb

EDSC-4252: fix some tests remove header and improve styling

3048a3b

EDSC-4252: Remove not needed components; clean up colormap, banner pl…

9c7848c

…acement, and scale-control placement - Revert banner to HEAD of main

EDSC-4252: fixing sizing of height; and removing extra renders

b3b1fbe

EDSC-4252: Fix rebase

fbcdc63

EDSC-4252: refactoring tests

930e602

EDSC-4252: fix more tests hotfix playwright test for now

f93edad

EDSC-4254: Fixing a few more things with the test

5f81096

EDSC-4252: Clean up; fix height issues when error banner was up

24bf829

EDSC-4252: Cleanup first pass

b699088

EDSC-4252: Cleaning up adding test and default value in the prop

c630dda

EDSC-4252: Fix playwright test start trying to fix the event.preventD…

eea580f

…efault test

EDSC-4252: Fix panel test, add css for tophat back in, add unit tests

1374924

EDSC-4252: Adding unit test fixing some playwright tests

86e571a

EDSC-4252: Cleanup temp disable map.spec to ensure its just that one …

56bab93

…that fails on GH actions

EDSC-4252: fix lint

78ca6e2

EDSC-4252: Fix search playwright test

9a2d288

EDSC-4252: Update color and spacing more cleanup

f8fb9fb

eudoroolivares2016 force-pushed the EDSC-4252 branch from fa9b961 to f8fb9fb Compare October 25, 2024 18:53

eudoroolivares2016 and others added 2 commits October 25, 2024 16:03

EDSC-4252: Comment map spec tests back in

a9a74f2

EDSC-4252: Fixes some map tests

b7358d8

eudoroolivares2016 requested review from trevorlang, bnp26, dpesall and daniel-zamora October 28, 2024 15:03

eudoroolivares2016 added 4 commits October 28, 2024 12:25

EDSC-4232: Adding wrapping container to fix banner mismatch tour is h…

af2238b

…aving issues

Update App.jsx

2267ded

EDSC-4232: Fix flaky panels test

a7290af

EDSC-4252: Cleaning up the container wrapper

c30a49a

eudoroolivares2016 force-pushed the EDSC-4252 branch from 046498c to c30a49a Compare October 29, 2024 16:35

eudoroolivares2016 added 2 commits October 29, 2024 12:53

EDSC-4252: fix lint

f844989

EDSC-4252: Clean up var names for secondaryToolbar && fix jest test

f4fcc04

eudoroolivares2016 commented Oct 29, 2024

View reviewed changes

trevorlang and others added 2 commits October 29, 2024 12:52

EDSC-4252: Fixing focus issues

5387523

EDSC-4252: Fix background color for not-found page; error page no lon…

50454b5

…ger needs logic, fix state for initial project value

eudoroolivares2016 commented Oct 29, 2024

View reviewed changes

eudoroolivares2016 and others added 6 commits October 29, 2024 16:24

Revert "EDSC-4252: Update values in search.spec.js"

b790463

This reverts commit cf03ea6.

EDSC-4252: Putting map styling with container wrapper in the correct …

dead856

…spot

EDSC-4252: fix some playwright tests

0da7bb0

EDSC-4252: fix lint forgot to save

b6945a8

EDSC-4252: Updating panel width max after conversation fixing more pl…

8ed6597

…aywright tests

EDSC-4252: Updates tests

14004f9

trevorlang requested changes Oct 30, 2024

View reviewed changes

EDSC-4252: Update comments from PR review

1888629

trevorlang approved these changes Oct 30, 2024

View reviewed changes

macrouch approved these changes Oct 30, 2024

View reviewed changes

eudoroolivares2016 and others added 2 commits October 30, 2024 13:40

EDSC-4252: fix comment; for now skip map tests these will be refined …

d846a38

…in subsequent ticket

EDSC-4252: Skip map tests

edf8de1

macrouch force-pushed the EDSC-4252 branch from 8c7bfdf to edf8de1 Compare October 30, 2024 18:09

eudoroolivares2016 merged commit 4b77b15 into main Oct 30, 2024
11 checks passed

eudoroolivares2016 deleted the EDSC-4252 branch October 30, 2024 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EDSC-4252: Update the Application header to match the design in post tophat2 update #1816

EDSC-4252: Update the Application header to match the design in post tophat2 update #1816

eudoroolivares2016 commented Oct 24, 2024 •

edited

Loading

codecov bot commented Oct 24, 2024 •

edited

Loading

eudoroolivares2016 Oct 25, 2024

eudoroolivares2016 Oct 25, 2024

eudoroolivares2016 Oct 29, 2024

eudoroolivares2016 Oct 29, 2024

trevorlang Oct 30, 2024

trevorlang Oct 30, 2024

trevorlang Oct 30, 2024

trevorlang Oct 30, 2024

eudoroolivares2016 Oct 30, 2024

trevorlang Oct 30, 2024

eudoroolivares2016 Oct 30, 2024

trevorlang Oct 30, 2024

eudoroolivares2016 Oct 30, 2024

trevorlang left a comment

trevorlang Oct 30, 2024

eudoroolivares2016 Oct 30, 2024

EDSC-4252: Update the Application header to match the design in post tophat2 update #1816

EDSC-4252: Update the Application header to match the design in post tophat2 update #1816

Conversation

eudoroolivares2016 commented Oct 24, 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 Oct 24, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevorlang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eudoroolivares2016 commented Oct 24, 2024 •

edited

Loading

codecov bot commented Oct 24, 2024 •

edited

Loading