fixed nested <a> error
AbandonedPublic
Actions

Authored by darn-it-dan on Jan 13 2022, 18:46.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

This revision is to fix a bug where we render a nested anchor tag in our <TxHistory /> component.

My strategy to fix the bug was:
replacing the surrounding the parent <a> tag with a styled <span> tag. (the styling replicates the styling of an <a> tag).
adding a role="link" to said span tag, to make it aware to screen readers that this is a span element with "link" functionality.

Test Plan

First we have to replicate the bug,
The easiest way is to make sure you have a wallet that recieved a transaction with a message.
The most obvious indicator of this is when you see a Reply To Message link in your transaction history.
Open up your console developer tools, and there should be NO red error message along the lines of:

Warning: validateDomNesting(...) <a> cannot appear as a descendant of <a>.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

nested-a-tags-fix

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 17917
Build 35657: Build Diff	cashtab-tests
Build 35656: arc lint + arc unit

Event Timeline

darn-it-dan created this revision.Jan 13 2022, 18:46

Owners added a reviewer: Restricted Owners Package.Jan 13 2022, 18:46

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 13 2022, 18:46

darn-it-dan requested review of this revision.Jan 13 2022, 18:46

Harbormaster completed remote builds in B17904: Diff 31798.Jan 13 2022, 18:49

Does not preserve current user experience

"Reply to" feature still works, but now the user will also open a tx in explorer.be.cash in a new tab at the same time as redirecting cashtab to reply to a message

web/cashtab/src/components/Wallet/TxHistory.js
15 ↗	(On Diff #31798)	Could be `const` instead of `function`
24 ↗	(On Diff #31798)	What is `data-testid` used for?

This revision now requires changes to proceed.Jan 13 2022, 23:04

Fixed UI behavior, reply button no longer opens explorer

Harbormaster completed remote builds in B17910: Diff 31808.Jan 14 2022, 02:54

changed function to const, deleted un-needed htmlAttribute

In D10826#242914, @bytesofman wrote:

Does not preserve current user experience

"Reply to" feature still works, but now the user will also open a tx in explorer.be.cash in a new tab at the same time as redirecting cashtab to reply to a message

whoops sorry about that, can't believe i missed that!

In D10826#242914, @bytesofman wrote:

Does not preserve current user experience

"Reply to" feature still works, but now the user will also open a tx in explorer.be.cash in a new tab at the same time as redirecting cashtab to reply to a message

also deleted data-testid, that was needed for a different revision I was working on, (got them confused).

and also changed function into const.

Harbormaster completed remote builds in B17911: Diff 31809.Jan 14 2022, 02:59

bytesofman requested changes to this revision.Jan 14 2022, 16:01

bytesofman added inline comments.

web/cashtab/src/components/Wallet/TxHistory.js
5 ↗	(On Diff #31809)	This is available as a global; check out `Send.js` to see how it is implemented without importing `{ theme }`
8 ↗	(On Diff #31809)	`color: ${props => props.theme.wallet.text.link};`

This revision now requires changes to proceed.Jan 14 2022, 16:01

refactoring <TxHistory> to use the global theme prop

Harbormaster completed remote builds in B17917: Diff 31816.Jan 14 2022, 17:28

emack mentioned this in D10833: [Cashtab] Copy to clipboard function for OP_RETURN messages.Jan 17 2022, 11:52

While this does work, I think it introduces an unnecessary complication that will need to be backed out later (passing a function to preserve the block explorer link functionality of the entire tx tile, which we want to replace with a more user-friendly 'external link' button on the tx tile). I think solving the issue per T1774 is the better approach. Please abandon this rev.

This revision now requires changes to proceed.Jan 19 2022, 20:43

darn-it-dan abandoned this revision.Jan 19 2022, 23:32