-
Notifications
You must be signed in to change notification settings - Fork 357
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
fix: [M3-8533] - Broken firewall rules table #11109
base: develop
Are you sure you want to change the base?
fix: [M3-8533] - Broken firewall rules table #11109
Conversation
Coverage Report: ✅ |
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.
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.
Good catch! 👍 I will update it.
edit: Done ✅
88dbf78
to
dd3015e
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 much better!
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.
Such styling discrepancy usually points at markup or composition problems.
In this case, the <FirewallRuleTable >
is very much a misnomer considering it renders div grid in place an actual HTML table. I am not why it was built that way (perhaps to account for the draggable aspect of its rows?) but I'd like explore implementing the right semantic markup instead of putting a styling band-aid. There would benefits doing so, namely consistency with our other features, styling and accessibility, as well as making sure this will update when we update any table related styling.
It increases the scope fo this particular PR, but worth exploring.
I agree that moving to more semantic markup for the I suggest we have a separate ticket to explore this implementation in detail. This way, we can ensure it receives the attention it deserves without delaying the current changes. Let me know what you think about this. |
@abailly-akamai I’ll explore this further and see how we can approach it effectively. Thanks! |
Description 📝
The table/list header row in the firewall rules table is not the correct color in dark/light mode. It is hard to tell that it is intended to be a table/list header.
Changes 🔄
PolicyRow
lg
, and with the last column for screens >=lg
.Target release date 🗓️
N/A
Preview 📷
lg
sm
andlg
sm
How to test 🧪
Reproduction steps
(How to reproduce the issue, if applicable)
Verification steps
As an Author I have considered 🤔
Check all that apply