EDSC-4162: Create a Guided Tour For Users of Earthdata Search #1800
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1800 +/- ##
==========================================
+ Coverage 93.38% 93.44% +0.05%
==========================================
Files 765 772 +7
Lines 18451 18543 +92
Branches 4761 4780 +19
==========================================
+ Hits 17231 17328 +97
+ Misses 1136 1131 -5
Partials 84 84 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
732 Lines is a huge component. To make it easier for devs to parse you could split out components into their own files (that would also make testing each component easier). You could also look at moving the steps into their own file, that seems like the vast majority of the actual lines.
| width: 4000px; | ||
| left: calc(100%); | ||
| pointer-events: none; | ||
| } |
| @@ -104,11 +103,20 @@ describe('Search component', () => { | |||
| expect(portalFeatureContainer.props().advancedSearch).toBeTruthy() | |||
| }) | |||
|
|
|||
| test('calls onTogglePortalBrowserModal(true) when "Browse Portals" button is clicked', () => { | |||
There was a problem hiding this comment.
Just curious if this was added for anything to do with the tour? Seems like a good test anyway
There was a problem hiding this comment.
For some reason, it was showing this was missing test coverage on my PR? So I just went ahead and added a test to cover it.
| </div> | ||
| ), | ||
| placement: 'right', | ||
| styles: { |
There was a problem hiding this comment.
we are gonna want to move alot of the repeated css into a classname also the bootstrap classes should be favored since we don't have to maintain them
static/src/js/App.jsx
Outdated
| const isSiteTourEnabled = disableSiteTour === 'false' | ||
| const hasUserDisabledTour = localStorage.getItem('dontShowTour') === 'true' | ||
| const isLocalhost = window.location.hostname === 'localhost' | ||
| const shouldShowTour = isSiteTourEnabled && !hasUserDisabledTour && !isLocalhost |
There was a problem hiding this comment.
Look into Playwright env variables to make the tour not run on each playwright test
| @@ -71,6 +73,7 @@ | |||
| margin-right: 0.5rem; | |||
| } | |||
|
|
|||
| &__begin-tour-button, | |||
There was a problem hiding this comment.
Rename to start-tour-button
| }, [runTour]) | ||
|
|
||
| useEffect(() => { | ||
| if (stepIndex === 6) { // Scrolling to the top to ensure "Browse Portals" is visible. |
|
|
||
| const handleJoyrideCallback = (data) => { | ||
| const { | ||
| action, index, status, type |
| { | ||
| options: { | ||
| primaryColor: '#007bff', | ||
| zIndex: 10000, | ||
| textAlign: 'left', | ||
| width: '600px' | ||
| }, | ||
| tooltip: { | ||
| fontSize: '16px', | ||
| padding: '20px', | ||
| paddingTop: '0px', | ||
| textAlign: 'left' | ||
| }, | ||
| tooltipContent: { | ||
| textAlign: 'left' | ||
| }, | ||
| buttonNext: { | ||
| // Hide the next button since we use custom buttons | ||
| display: 'none' | ||
| } |
There was a problem hiding this comment.
rem instead of pixels, try to find a way to use class names instead of inline styling
| floaterProps={ | ||
| { | ||
| disableAnimation: true, | ||
| styles: { | ||
| button: { | ||
| borderRadius: '4px', | ||
| padding: '8px 16px', | ||
| fontSize: '14px' | ||
| } | ||
| } | ||
| } | ||
| } | ||
| /> |
| { | ||
| fontSize: '16px', | ||
| fontWeight: 'bold', | ||
| marginBottom: '5px' | ||
| } |
| styles: { | ||
| tooltip: { | ||
| width: '600px', | ||
| padding: '20px' | ||
| }, | ||
| tooltipContent: { | ||
| fontSize: '16px' | ||
| } | ||
| } |
| await expect(page.locator('.tour-content').first()).toContainText('This area contains the filters used when searching for collections') | ||
|
|
||
| // Move forward to the next step again using the "Next" button | ||
| await page.locator('.tour-buttons button:has-text("Next")').click() |
There was a problem hiding this comment.
Try removing class names from use by locator
| for (let i = 0; i < 10; i += 1) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await page.locator('.tour-buttons button:has-text("Next")').click() | ||
| } |
There was a problem hiding this comment.
Make sure to check that each step is highlighting the proper section
| // Ensure the tour is closed by checking that the tour container is no longer visible | ||
| await expect(page.locator('.tour-container')).toBeHidden() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Add test to ensure the tour starts up for a brand new user by default
Overview
What is the feature?
Creating a guided tour for EDSC users
What is the Solution?
Creating a tour using React-Joyride
What areas of the application does this impact?
EDSC Search Endpoint
Testing
Reproduction steps
Attachments
Please include relevant screenshots or files that would be helpful in reviewing and verifying this change.
Checklist