-
Notifications
You must be signed in to change notification settings - Fork 2
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
refactor: ui tweak to make badge position fixed #232
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
@ss-dimasm can you please add screenshots showing before and after? 🙏 |
@kurtdoherty , this is the visual; the badge position is more cornered than before left is in this PR, right is in and it will also have the flex: 1 1 0 property to automatically fill available space within a flex container |
@kurtdoherty, hold on I surprised the total files changed is 32 and it's more than 1000 lines 😱😱, fixing |
@ss-dimasm thanks for the details 🙏 This PR currently includes a bunch of changes that I think belong to other branches we've merged in. If you aren't already rebasing your branch onto latest, can you please do so. Ideally, this branch only includes the changes relevant to it; not historical changes that have been pulled in from main. |
4b11bf1
to
0b9d384
Compare
apologies @kurtdoherty, I didn't notice while rebasing |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
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.
LGTM 👍
<ElBottomBarItemBadge role="status" /> | ||
<ElBottomBarItemContent> | ||
<ElBottomBarItemIcon>{args?.icon}</ElBottomBarItemIcon> | ||
<ElBottomBarItemBadge role="status" /> |
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.
nit-pick: I don't think status
is an appropriate ARIA role. I'm not sure what is, but when I read the MDN docs on the status role, it doesn't feel quite right 🤷
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.
I don't think status is an appropriate ARIA role.
ah yes, I forgot to remove it for storybook styles usage after we've agree on this feedback
Pull request checklist
Detail as per issue below (required):
fixes: #
part of #192, minor UI tweak for
BottomBarItem
. The badge position is breaks when composed in wider components and a little typo font size usage on there