Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(layout): add patternfly to layout #13

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

kahboom
Copy link
Collaborator

@kahboom kahboom commented Feb 15, 2024

tldr: This PR implements SECURESIGN-586 to add PatternFly to the layout, leaving subcomponents alone (to follow in a separate PR shortly). Feel free to leave unmerged until the second PR with subcomponents if you do not want to merge it partially implemented (see screenshots below).

Context:
I was originally wanting to follow an approach where we can map to the upstream code by wrapping the components, such that they can support modifications, to make diffing clearer and maintenance easier when changes are inevitably made upstream, but this was taking too much time and ultimately was not as clean as I had hoped and would otherwise be over-engineered. There might also have a performance hit given that the components are not leveraging React server-components and would need to be rendered by the browser, so manually swapping out Material UI for PatternFly seems to be the best solution (for now).

Changes:

  • adds PatternFly react and core to the code base
  • resolves issue with Next.js + PF, where Next.js does not allow packages to import styles from their subdependencies (globally)
  • adds RHTAS branding

A future PR can improve on this code by adding unit (and e2e?) tests, which upstream does not have, and possibly mapping components to sections in the upstream UI rather than 1:1 component mapping.

Screenshots

Empty state:

Empty state

Search results:

Results

Settings:

Settings

Copy link

@Gregory-Pereira Gregory-Pereira left a comment

Choose a reason for hiding this comment

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

No other comments, very clean. Haven't tested it myself but if your confident then lets go with this.
/lgtm

public/github.svg Outdated Show resolved Hide resolved
@kahboom kahboom merged commit 43170f8 into securesign:main Feb 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants