EES-5496 Fix inconsistent anchor IDs causing broken nav links in API data set mapping pages #5481
Conversation
ntsim
changed the title
| import compact from 'lodash/compact'; | ||
|
|
||
| export const sectionIds = { |
There was a problem hiding this comment.
Seems like a neat way of keeping the nav and the links consistent. Do we do this pattern elsewhere at all?
| > | ||
| New filter options ({totalNewFilterOptions}){' '} | ||
| <Tag colour="grey">No action required</Tag> | ||
| </h3> | ||
|
|
||
| {totalNewFilterOptions > 0 && filtersMapping ? ( | ||
| <Accordion | ||
| id="new-filter-options-accordion" | ||
| testId="new-filter-options-accordion" | ||
| id={`${sectionIds.newFilterOptions}-accordion`} |
There was a problem hiding this comment.
Worth having a sectionIds.newFilterOptionsAccordion variable just to avoid the risk of this too getting out of sync?
| > | ||
| Auto mapped filter options ({totalAutoMappedFilterOptions}){' '} | ||
| <Tag colour="grey">No action required</Tag> | ||
| </h3> | ||
|
|
||
| {totalAutoMappedFilterOptions > 0 ? ( | ||
| <Accordion | ||
| id="auto-mapped-filter-options-accordion" | ||
| id={`${sectionIds.autoMappedFilterOptions}-accordion`} |
There was a problem hiding this comment.
As mentioned before, worth a variable for this? Same comments for other such changes in this PR :)
| className="dfe-table--vertical-align-middl" | ||
| data-testid={`auto-mapped-table-${groupKey}`} | ||
| className="dfe-table--vertical-align-middle" | ||
| data-testid={tableId} |
There was a problem hiding this comment.
Worth giving the table's id attribute this value as well as the data-testid, like the table in ApiDataSetMappableTable.tsx? Or is it purely that the other table needs to have an id for anchor links to target it?
| @@ -163,7 +163,7 @@ Validate the version task statuses inside the 'Draft version task' section | |||
| User clicks on Map locations link | |||
| user clicks link Map locations | |||
| user waits until h3 is visible Locations not found in new data set | |||
| user waits until element contains xpath://table[@data-testid='mappable-table-region']/caption//strong[1] | |||
| user waits until element contains css:[data-testid="mappable-table-region"] caption | |||
There was a problem hiding this comment.
Was looking to find Robot changes :) Nice one - looks like a good simplification too
This PR fixes inconsistent anchor IDs being used across the the public API filters and locations mapping pages. Previously, we were using hard-coded ID strings meaning that it was easy to forget to update them in all places. This would then lead to broken navigation links due to stale IDs being used.
We've not refactored both the filters and locations mappings pages to use common variables and functions to generate the necessary anchor IDs. This should hopefully keep things consistent and less bug-prone moving forward.