Skip to content

refactor(web): UI code pruning and clean up round #2#1494

Merged
dgdavid merged 16 commits intomasterfrom
frontend-cleanup-2
Jul 29, 2024
Merged

refactor(web): UI code pruning and clean up round #2#1494
dgdavid merged 16 commits intomasterfrom
frontend-cleanup-2

Conversation

@dgdavid
Copy link
Copy Markdown
Contributor

@dgdavid dgdavid commented Jul 23, 2024

Related to #1441, just another round of UI code improvements and clean up.

@dgdavid dgdavid requested review from imobachgs and teclator July 23, 2024 15:56
@dgdavid

This comment was marked as outdated.

dgdavid added 4 commits July 24, 2024 17:53
By moving them from ~/components/{namespace}/routs.js to
~/routes/{namespace}.tsx

It also adds a constant PATHS per namespace and uses them instead of
directly typing the route path again when needed.
Using it as a wrapper for PF/PageGroup accepting children instead of a
wrapper for a router <Outlet>. The latest is already rendered by the
layout.

Please, not that this change is just the starting point of a bigger Page
component refactor that should help to improve both, the DX experience
and the resulting DOM output.
@dgdavid dgdavid force-pushed the frontend-cleanup-2 branch from 9386bbe to 6c21d01 Compare July 24, 2024 16:54
dgdavid added 11 commits July 26, 2024 07:55
Because Loading component should be an standalone component agnostic to
the layout. Moreover, the SimpleLayout is already importing the Loading
component which was kind of circular dependency.
By using functions instead of variables in router.js to prevent the
execution of code while importing the file.
Needed to avoid type errors in subsequent changes because not known
types of `handle` object values.
For wrapping the Loading component with SimpleLayout.

Related to 888830a
Although it could be drop in a future.
@dgdavid dgdavid marked this pull request as ready for review July 29, 2024 12:49
@dgdavid dgdavid changed the title web: UI code pruning and clean up round #2 refactor(web): UI code pruning and clean up round #2 Jul 29, 2024
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 29, 2024

Coverage Status

coverage: 70.959%. remained the same
when pulling 01b81ad on frontend-cleanup-2
into 59cb9b4 on master.

@dgdavid dgdavid merged commit 55d8a70 into master Jul 29, 2024
@dgdavid dgdavid deleted the frontend-cleanup-2 branch July 29, 2024 21:45
dgdavid added a commit that referenced this pull request Aug 13, 2024
…ttons (#1536)

#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.
dgdavid added a commit that referenced this pull request Sep 13, 2024
**Apart from a bit of clean up, this PR is intended for start writing
better core components** that has been on hold for a few months already.

It's the case of _core/Page_ component, which has been rewritten almost
for scratch and now makes the weird _core/CardField transitioning
component_ obsolete.


Please, note that this set of changes **continues with the migration to
TypeScript for touched files** and also **introduce a PatternFly/Flex
wrapper** in order to ease the work with its responsive props. It's a
bit complex because the (ab)use of advanced types but it does the job
without introducing props unknown by PF/Flex. As said in the file
comments, ideally

> would be better to add these responsive props shortcuts direclty in
PF/Flex to allow the consumer to just set the `default` value when not
needed to change it depending on the breakpoint. But at this moment
we're a bit short of time for creating and testing such an elaborated PR
against upstream.

---

Related to #1441 and
#1494
@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.

3 participants