Skip to content

fix(web): restore look&feel of HTML a tags pretending to look as buttons#1536

Merged
dgdavid merged 8 commits intomasterfrom
fix-link-buttons-style
Aug 13, 2024
Merged

fix(web): restore look&feel of HTML a tags pretending to look as buttons#1536
dgdavid merged 8 commits intomasterfrom
fix-link-buttons-style

Conversation

@dgdavid
Copy link
Copy Markdown
Contributor

@dgdavid dgdavid commented Aug 13, 2024

#1494 introduced a typo when migrating former and temporary src/components/core/ButtonLink to TypeScript, making it to stop looking as a button.

Although the fix was ridicously simple, just adding the missing e to scondary, this PR takes the opportunity for improving the whole component, getting ride of a pending FIXME, using a better name for it, and also adding simple but useful unit tests.

** Reviewers: going commit by commit could make easier the review.

Before After
Screenshot from 2024-08-13 15-32-37 Screenshot from 2024-08-13 15-32-08
Screenshot from 2024-08-13 15-32-32 Screenshot from 2024-08-13 15-32-13

There were a typo in "secondary" making links does not appears as
secondary PF/Buttons
To use a PF/Button and a ReactRouter/useNavigate hook instead of the
ReactRouter/Link component. It helps to simplify the code and easily
keep a visual consistence.
Because the MemoryRouter used for testing is only mounted when unsing
installerRender. Most probably, something to review in a future
iteration of testing improvements.
@dgdavid dgdavid requested a review from ancorgs August 13, 2024 14:56
For some reason, the test worked as it was locally but failed at CI.
Copy link
Copy Markdown
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dgdavid dgdavid merged commit 0f1a1d5 into master Aug 13, 2024
@dgdavid dgdavid deleted the fix-link-buttons-style branch August 13, 2024 15:21
dgdavid added a commit that referenced this pull request Aug 14, 2024
#1536 faced some problems in CI
not reproducible locally resulting in a set of changes in the test suite
to make CI happy.

But these CI complaints were right since a mock for ReactRouter#useHref
hook was forgotten and not sent as part of the changes, reason why
initial tests were not working.
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
imobachgs added a commit that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants