feat(NcAppSidebarTab): Redesign active tab as rounded filled pill#8447
feat(NcAppSidebarTab): Redesign active tab as rounded filled pill#8447
Conversation
39f2471 to
9056878
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8447 +/- ##
==========================================
+ Coverage 54.55% 54.57% +0.02%
==========================================
Files 106 106
Lines 3439 3441 +2
Branches 1002 1002
==========================================
+ Hits 1876 1878 +2
Misses 1322 1322
Partials 241 241 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9056878 to
1ab6511
Compare
susnux
left a comment
There was a problem hiding this comment.
What about backwards compatibility with old servers?
This likely needs legacy workarounds just like we did with the border rework we recently did. So that apps on e.g. NC32 still have the old design.
|
(I removed the icon and added font-weight to make it closer to the draft on the screenshot)
Another thing that catches my eyes — how it lies on the gray line between the tablist and the tab content. Previously it looked fine to me, but now with the border radius and the gap if feels like the gap is missing between the buttons and the content, or not necessary at all.
Also, I'm not sure what it is supposed to look like with the icon. On the draft there are no icons. With the current size, it looks unbalanced to me, with the icon too close to the top.
cc @kra-mo |
e97cd55 to
6b2a7e2
Compare
6b2a7e2 to
bb2edfc
Compare
Yeah, I see what you mean, some more padding would be nice. |
|
It would be good to have the |
|
@kra-mo What about the other points from above?
|
I was thinking a bit, and honestly, it being wider works in more cases so maybe it's better to just go with that.
It does look better if it's a bit thicker, but 4px is a variable, I'd say maybe 6px (1.5x).
On the bottom, no gap.
Yes, elements with border radius should not hug hard edges. We should add a bit of a gap :) |
bb2edfc to
2bed8ec
Compare
| renderIcon() { | ||
| return this.$slots.icon?.() | ||
| renderIcon(selected = false) { | ||
| return this.$slots.icon?.({ selected }) |
There was a problem hiding this comment.
Although the new param's name reflects HTML (aria-selected) and the current internal variable name, I'm not sure about the naming:
- Parent's selected tab is defined by
activeprop, which results in 2 names for the same thing (The current tab isactiveand whether the tab is active isselected) - Boolean flag pattern seems more fitting here, e.g.
isSelected/isActive. And it also would allow in the future having both the boolean flag and the value if needed.
Note: this changes the component API and refactoring it later would be only possible with backward compatibility and migration.
The new prop might not be required for this feature. There is an active prop already.
It is slightly more complicated to use, though.
<NcAppSidebar v-model:active="active">
<!-- ... -->
<NcAppSidebarTab name="Settings" id="settings-tab">
<template #icon>
<IconCog v-if="active === 'settings-tab'" :size="20" />
<IconCogOutline v-else :size="20" />
</template>
</NcAppSidebarTab>
</NcAppSidebar>cc @susnux
| gap: var(--default-grid-baseline); | ||
| margin: 10px 8px 0 8px; | ||
| padding-block-end: var(--default-grid-baseline); |
2bed8ec to
82c1372
Compare
The active sidebar tab now renders as a fully-rounded filled pill with an inset, rounded blue indicator that grows into place when selected, replacing the previous full-width bottom border and bold label treatment. Tabs keep a small gap between each other to eliminate the hover radius artifact, and the tab icon slot now receives the active state so consumers can render filled/outlined icon variants. Signed-off-by: nfebe <fenn25.fn@gmail.com>
82c1372 to
a83c6d0
Compare







🖼️ Screenshots
Screencast
active-tabs-demo-7520.webm
Closes #7520