-
Notifications
You must be signed in to change notification settings - Fork 16
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
Upgrade to Patternfly v6 #1191
base: main
Are you sure you want to change the base?
Upgrade to Patternfly v6 #1191
Conversation
ccb4580
to
726cad9
Compare
b30e874
to
de5ce70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice @hemahg. Here are a few issues I've seen browsing around the app so far. Also, if it is simple, it might be a good option to allow a toggle for light/dark modes [1] like
[1] https://www.patternfly.org/developer-resources/dark-theme-handbook
Brokers heading shifted up and left. Also, the rebalance tab is missing and this image is for a managed Kafka with a CR.
Same heading issue for an individual topic, and the selected tab isn't highlighted:
Partition tabs are transparent - scrolling the configuration is visible through the tabs:
1.Fixing the Navigation and Highlight Issues: The heading issue in the Topic and Broker tabs has been addressed. |
Ignore what I said earlier about the missing rebalance tab - it was an issue with my configuration. Sorry for any confusion on that. |
Got it. Maybe we can switch to using the tab component in the future [1]? Did we have a reason to use nav instead of tab here to begin with? cc: @riccardo-forina |
At the time, the tabs were full client components that did not support the routing at all. We have a unique url tied to each tab that loads the new portion of data following the way next.js router works. So I couldn't use that. And the tabs weren't a great fit estically speaking. Feel free to change them to whatever works better. |
Ok, I see what you mean. It looks like there is support in PF6 for "tabs linked to Nav elements". We can think about whether that makes sense for a future PR. https://www.patternfly.org/components/tabs#tabs-linked-to-nav-elements |
That option was already available, but it renders plain html links, not Next Link components, unfortunately. I suppose we could still use that, and add an onClick handler that triggers the react navigation programmatically preventing the default behavior. Not ideal, but workable I suppose |
Signed-off-by: hemahg <[email protected]>
Signed-off-by: hemahg <[email protected]>
Signed-off-by: hemahg <[email protected]>
Signed-off-by: hemahg <[email protected]>
Signed-off-by: hemahg <[email protected]>
5116446
to
302818d
Compare
Signed-off-by: hemahg <[email protected]>
Quality Gate passedIssues Measures |
Upgrade to Patternfly v6