feat: export TabPanel #6896

bvandercar-vt · 2024-07-05T17:59:56Z

Fixes #0000

Checklist

Includes tests
Update documentation

Changes proposed in this pull request:

Extract the tab panel wrapper from renderTabPanel to its own component that is exported. This is useful when rendering a tab panel when using Tabs in controlled mode.

changelog-app · 2024-07-05T18:05:00Z

Generate changelog in `packages/core/changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

feat: TabPanel

Check the box to generate changelog(s)

Generate changelog entry

packages/core/test/tabs/tabsTests.tsx

…into develop

…develop

evansjohnson

Some comments here but I'm generally in support of this. There's not too much this component does, but it hooks up some accessibility roles that are otherwise liable to not be set.

evansjohnson · 2024-07-15T20:19:46Z

packages/core/src/components/tabs/tabTitle.tsx

+export function generateTabIds(parentId: TabId, tabId: TabId) {
+    return {
+        tabPanelId: `${Classes.TAB_PANEL}_${parentId}_${tabId}`,
+        tabTitleId: `${Classes.TAB}-title_${parentId}_${tabId}`,
+    } satisfies TabIdProps;


There's not too much this component does, but it hooks up some accessibility roles that are otherwise liable to not be set.

Bingo, exactly!

evansjohnson · 2024-07-15T20:19:58Z

packages/core/changelog/@unreleased/pr-6865.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+fix:
+  description: '[core] a11y(Tooltip): wrap contents in "tooltip" aria role'


evansjohnson · 2024-07-15T20:23:26Z

packages/core/src/components/tabs/tabs.tsx

@@ -325,25 +326,19 @@ export class Tabs extends AbstractPureComponent<TabsProps, TabsState> {
    }

    private renderTabPanel = (tab: TabElement) => {
-        const { className, panel, id, panelClassName } = tab.props;
+        const { className, id, panel, panelClassName } = tab.props;


nit: no diff here please

evansjohnson · 2024-07-15T20:29:26Z

packages/core/src/components/tabs/tabPanel.tsx

+    /**
+     * Used for setting `aria-hidden` prop.
+     */


Suggested change

/**

* Used for setting `aria-hidden` prop.

*/

/**

* If true, will set required accessibility roles and set `display` to `none`.

*/

I'm thinking maybe we just make this required if the expected usage is that one TabPanel is provided for each tab, like is done in the uncontrolled usage of Tabs. In your example you essentially share a TabPanel but I'm wondering if making this optional encourages that pattern.

packages/docs-app/src/examples/core-examples/tabsExample.tsx

packages/core/test/tabs/tabsTests.tsx

evansjohnson · 2024-07-15T20:37:06Z

packages/core/src/components/tabs/tabPanel.tsx

+import { type TabProps } from "./tab";
+import { generateTabIds, type TabTitleProps } from "./tabTitle";
+
+export interface TabPanelProps extends Pick<TabProps, "className" | "id" | "panel">, Pick<TabTitleProps, "parentId"> {


re panel - do we want to inherit the If omitted, no panel will be rendered for this tab. Can either be an element or a renderer. behavior?

if so we probably want something similar to https://github.com/palantir/blueprint/pull/6896/files#diff-cf85fdaa862aad83538297ecd6e3a3b6a5a3fee30bd8aaaea4a2294f462c7933R330-R332 where we early return if panel is undefined

Yep. I had to remove TabPanel from the isomorphic tests then. 8f202ae

evansjohnson · 2024-07-15T20:53:22Z

packages/core/src/components/tabs/tabPanel.tsx

+import { type TabProps } from "./tab";
+import { generateTabIds, type TabTitleProps } from "./tabTitle";
+
+export interface TabPanelProps extends Pick<TabProps, "className" | "id" | "panel">, Pick<TabTitleProps, "parentId"> {


I wonder if we should also support the renderActiveTabPanelOnly prop here and do an early return when isHidden as a convenience without needing to set that up yourself and to keep the same feature set offered by uncontrolled tabs

If someone is using TabPanel and wants to render active tab panel only, they would be handling the visibility logic themselves outside of the TabPanel component.

But it wouldn't hurt anything to add your reccomendation as a bonus 🤷

done fc9d945

…develop

…t/blueprint into bvandercar/tabPanel

evansjohnson

we're you able to figure out getting TabPanel to show up in the docs? just one more small suggestion otherwise curious to hear your thoughts on it then this one looks ready to go

evansjohnson · 2024-07-16T20:07:01Z

packages/core/src/components/tabs/tabs.tsx

-                {Utils.isFunction(panel) ? panel({ tabTitleId, tabPanelId }) : panel}
-            </div>
+                className={classNames(className, panelClassName)}
+                isHidden={id !== this.state.selectedTabId}


since any consumer would need to set this same logic up I wonder if we should have the prop be selectedTabId instead so that users can't do weird things like have multiple tab panels showing

Eh the logic may not always be the same. Take, for example, this case where isHidden is always false because there's always only 1 TabPanel being rendered:

import * as React from "react"; import { Tab, Tabs, TabPanel, type TabId } from "@blueprintjs/core"; ... const TABS_PARENT_ID = React.useId(); const [selectedTabId, setSelectedTabId] = React.useState<TabId>("Home"); <Tabs id={TABS_PARENT_ID} onChange={setSelectedTabId} selectedTabId={selectedTabId} > <Tab id="Home" title="Home" /> <Tab id="Files" title="Files" /> </Tabs> ... <TabPanel id={selectedTabId} isHidden={false} parentId={TABS_PARENT_ID} panel={<p>The current panel id is: "{selectedTabId}"</p>} />

But then again, if we changed it to selectedTabId they could just have it be

<TabPanel id={selectedTabId} selectedTabId={selectedTabId} parentId={TABS_PARENT_ID} panel={<p>The current panel id is: "{selectedTabId}"</p>} />

I'm down for either, let me know which you like better.

yea I think I'm leaning towards selectedTabId because then we control more of the implementation - I'm worried about trying to use the same TabPanel for multiple tabs causing issues down the line somehow

Done 12a665b

…develop

bvandercar-vt · 2024-07-23T21:42:10Z

we're you able to figure out getting TabPanel to show up in the docs?

Just got that done!

evansjohnson · 2024-07-25T16:33:24Z

packages/docs-app/src/examples/core-examples/tabsExample.tsx

+                        id={this.state.navbarTabId}
+                        selectedTabId={this.state.navbarTabId}


lol I guess I didn't see this coming where id could just also be dynamic 🤷

that's fine I can't think of an easy way to prevent this and maybe it is okay. if we ever actually see an accessibility issue with changing the id dynamically we can update the docs at that time but no need to go searching for if this would be an issue

packages/core/src/components/tabs/tabPanel.tsx

evansjohnson

LGTM - thanks for the contribution and getting the docs updated! 🎉

bvandercar-vt added 6 commits June 24, 2024 22:47

add changelog entry

73a0519

feat: tabPanel renderer

4ac5b19

refactor: generateTabIds

6d456b5

remove rename

316f4a5

export type

7d1be88

fix order

83116ef

bvandercar-vt added 10 commits July 5, 2024 12:12

style: return undefined

0a007a1

refactor: isVisible prop

45d761a

style: jsdocs

bcdb95b

Merge branch 'develop' into bvandercar/tabPanel

c6b3717

remove return undefined

087ac23

move key

c2aed65

test: improve tabPanels tests

9adf944

Merge branch 'bvandercar/test/tabs' into bvandercar/tabPanel

5472c30

fix test

1f2d078

style: use const

fdf8652

bvandercar-vt commented Jul 8, 2024

View reviewed changes

packages/core/test/tabs/tabsTests.tsx Outdated Show resolved Hide resolved

bvandercar-vt added 4 commits July 8, 2024 11:33

style: date

5b91852

add example

b773ae3

example: use section

75a6efc

Merge branch 'develop' of https://github.com/bvandercar-vt/blueprint …

241696b

…into develop

bvandercar-vt changed the title ~~feat: TabPanel~~ feat: export TabPanel Jul 9, 2024

bvandercar-vt added 2 commits July 12, 2024 08:12

Merge branch 'develop' of https://github.com/palantir/blueprint into …

150687f

…develop

Merge branch 'develop' into bvandercar/tabPanel

f8794af

evansjohnson reviewed Jul 15, 2024

View reviewed changes

evansjohnson self-assigned this Jul 15, 2024

evansjohnson reviewed Jul 15, 2024

View reviewed changes

Merge branch 'develop' of https://github.com/palantir/blueprint into …

110923a

…develop

bvandercar-vt mentioned this pull request Jul 16, 2024

test: improve tabs tests #6898

Merged

1 task

bvandercar-vt and others added 6 commits July 16, 2024 08:16

Delete packages/core/changelog/@unreleased/pr-6865.v2.yml

955d4f1

revert reorder

aabcc6b

Merge branch 'bvandercar/tabPanel' of https://github.com/bvandercar-v…

dc3f729

…t/blueprint into bvandercar/tabPanel

require isHidden

7881c58

return undefined if panel undefined

8f202ae

pick renderActiveTabPanelOnly prop

fc9d945

evansjohnson reviewed Jul 16, 2024

View reviewed changes

bvandercar-vt added 4 commits July 23, 2024 14:56

Merge branch 'develop' of https://github.com/palantir/blueprint into …

2254a18

…develop

Merge branch 'develop' into bvandercar/tabPanel

3fbc070

add TabPanel example

9f9c0b6

uncontrolled usage example

6b820bd

bvandercar-vt added 3 commits July 23, 2024 15:54

style: tabs example

0bd052b

fix test

72bfa73

change isHidden to selectedTabId

12a665b

evansjohnson reviewed Jul 25, 2024

View reviewed changes

Update packages/core/src/components/tabs/tabPanel.tsx

01cb908

evansjohnson approved these changes Jul 25, 2024

View reviewed changes

evansjohnson enabled auto-merge (squash) July 25, 2024 16:38

gluxon approved these changes Jul 25, 2024

View reviewed changes

evansjohnson merged commit 6491c01 into palantir:develop Jul 25, 2024
5 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: export TabPanel #6896

feat: export TabPanel #6896

bvandercar-vt commented Jul 5, 2024 •

edited

Loading

changelog-app bot commented Jul 5, 2024

evansjohnson left a comment

evansjohnson Jul 15, 2024

bvandercar-vt Jul 16, 2024

evansjohnson Jul 15, 2024

evansjohnson Jul 15, 2024

evansjohnson Jul 15, 2024

evansjohnson Jul 15, 2024

bvandercar-vt Jul 16, 2024

evansjohnson Jul 15, 2024

bvandercar-vt Jul 16, 2024 •

edited

Loading

evansjohnson Jul 15, 2024

bvandercar-vt Jul 16, 2024

bvandercar-vt Jul 16, 2024

bvandercar-vt Jul 16, 2024

evansjohnson left a comment

evansjohnson Jul 16, 2024

bvandercar-vt Jul 23, 2024 •

edited

Loading

evansjohnson Jul 24, 2024

bvandercar-vt Jul 25, 2024

bvandercar-vt commented Jul 23, 2024

evansjohnson Jul 25, 2024

evansjohnson left a comment

		id={this.state.navbarTabId}
		selectedTabId={this.state.navbarTabId}

feat: export TabPanel #6896

feat: export TabPanel #6896

Conversation

bvandercar-vt commented Jul 5, 2024 • edited Loading

Fixes #0000

Checklist

Changes proposed in this pull request:

changelog-app bot commented Jul 5, 2024

Generate changelog in packages/core/changelog/@unreleased

evansjohnson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvandercar-vt Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evansjohnson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvandercar-vt Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvandercar-vt commented Jul 23, 2024

Choose a reason for hiding this comment

evansjohnson left a comment

Choose a reason for hiding this comment

bvandercar-vt commented Jul 5, 2024 •

edited

Loading

Generate changelog in `packages/core/changelog/@unreleased`

bvandercar-vt Jul 16, 2024 •

edited

Loading

bvandercar-vt Jul 23, 2024 •

edited

Loading