Page MenuHomePhabricator

[e.cash] Add id param and better comments to H3 component
ClosedPublic

Authored by johnkuney on Jun 14 2023, 19:49.

Details

Summary

Adding some better comments and an optional id paramater to the H3 component.
This will allow for adding a CSS id to any H3, which will be needed for some anchor links in other diffs
Also added method to verify its a proper id and test to test it

Test Plan

npm run dev
go to core-tech page
inspect element on the 'AVALANCHE' title, and check it has no id assigned to it
Then add an id={title} to the H3 instance in TextImageBlock on the core-tech page
inspect the element again and should see id="AVALANCHE" on the h3
then npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ecash-id-to-h3
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 24046
Build 47702: Build Diff
Build 47701: arc lint + arc unit

Event Timeline

bytesofman added a subscriber: bytesofman.
bytesofman added inline comments.
web/e.cash/components/h3/index.js
15 ↗(On Diff #40793)

what happens if id is undefined?

This revision now requires changes to proceed.Jun 14 2023, 20:00

Nothing actually if its undefined, but it will put a blank id if an empty string gets passed which is invalid, so adding back in that check

This revision is now accepted and ready to land.Jun 14 2023, 20:31
johnkuney edited the test plan for this revision. (Show Details)
web/e.cash/components/h3/isValidCSSId.js
9

too much lol

means the page can still load if a dev gives this component a bad ID ... but in this case, the component just has no ID, which is not what the dev expects

Could be worth adding snapshot testing to make sure the component renders even if id is not provided. But, since

  • This is just a web page and not a complicated app
  • The failure mode can only happen in active development, i.e. build would fail if a diff breaks this

...even this would be a bit over-engineered imo

johnkuney edited the test plan for this revision. (Show Details)

remove check