feat(tabs): Add last-tab-closable functionality - #14446
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a lastTabClosable option for calcite-tabs so apps can preserve the close affordance when only one visible closable tab remains.
Changes:
- Adds
lastTabClosabletocalcite-tabsand forwards it tocalcite-tab-nav. - Updates tab-nav close-button hiding/restoration logic.
- Adds an e2e test for preserving the last close button.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/components/src/components/tabs/tabs.tsx |
Defines and propagates the new tabs property. |
packages/components/src/components/tab-nav/tab-nav.tsx |
Implements last-visible-tab close button behavior and selection handling updates. |
packages/components/src/components/tabs/tabs.e2e.ts |
Adds coverage for retaining the close button with lastTabClosable. |
I don’t see why we can’t keep this option around for a while and gather feedback before making further adjustments. WRT changing the default behavior, the request to change the tab closing behavior recently came up since the feature was introduced in v1.4.0, so I have a hunch this may not be the majority use case. Curious if others have seen stronger demand for making this the default and setting up for removal later. cc @Esri/calcite-pes |
I have not seen the demand, but others may have (re-pinging for timeliness) cc @Esri/calcite-pes. Even if we keep it around for a while, we can open the issue to make it the default behavior where the reasoning could be documented which would be helpful when asking teams. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
0637f87 to
bb5bf59
Compare
jcfranco
left a comment
There was a problem hiding this comment.
Awesome! 🚀 Other than a few comments, this LGTM!
| return; | ||
| } | ||
|
|
||
| expect(reopenedTitle.closed).toBe(false); |
There was a problem hiding this comment.
Suggestion: depending on how much direct element access is needed, locators could help simplify:
const reopenedTitle = page.locator("#tab-title-4");
const reopenedTab = page.locator("#tab-4");
await expect.element(reopenedTitle).toBeInTheDocument();
const reopenedTitleEl = await reopenedTitle.element() as TabTitle["el"];
reopenedTitleEl.closed = false;
await component.updateComplete;
await expect.element(reopenedTab).toBeInTheDocument();
await expect.element(reopenedTitle).toHaveProperty("closed", false);
await expect.element(reopenedTitle).toHaveProperty("hidden", false);
await expect.element(reopenedTitle).toHaveProperty("tabIndex", 0);
await expect.element(reopenedTab).toHaveProperty("hidden", false);
await expect.element(reopenedTab).toHaveProperty("selected", true);
Related Issue: #14346
Summary
This adds a property to allow the last
closableTab to be closed.We need to open a
future breaking changeissue to make this the default behavior. At that time, we should deprecate this newly addedlast-tab-closable.