Skip to content
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: Update st.tabs visuals to make stale tabs more obvious #7310

Merged
merged 5 commits into from Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 22 additions & 12 deletions frontend/lib/src/components/elements/Tabs/Tabs.test.tsx
Expand Up @@ -15,11 +15,13 @@
*/

import React from "react"
import { mount } from "@streamlit/lib/src/test_util"
import "@testing-library/jest-dom"

import { screen, within } from "@testing-library/react"
import { render } from "@streamlit/lib/src/test_util"
import { BlockNode } from "@streamlit/lib/src/AppNode"
import { Block as BlockProto } from "@streamlit/lib/src/proto"

import { Tabs as UITabs } from "baseui/tabs-motion"
import Tabs, { TabProps } from "./Tabs"

function makeTab(label: string, children: BlockNode[] = []): BlockNode {
Expand Down Expand Up @@ -47,26 +49,34 @@ const getProps = (props?: Partial<TabProps>): TabProps =>

describe("st.tabs", () => {
it("renders without crashing", () => {
const wrapper = mount(<Tabs {...getProps()} />)
expect(wrapper.find(UITabs).exists()).toBe(true)
expect(wrapper.find("StyledTab").length).toBe(5)
render(<Tabs {...getProps()} />)

const tabsContainer = screen.getByRole("tablist")
expect(tabsContainer).toBeInTheDocument()
const tabs = within(tabsContainer).getAllByRole("tab")
expect(tabs).toHaveLength(5)
})

it("sets the tab labels correctly", () => {
const wrapper = mount(<Tabs {...getProps()} />)
render(<Tabs {...getProps()} />)
const tabs = screen.getAllByRole("tab")
expect(tabs).toHaveLength(5)

expect(wrapper.find("StyledTab").length).toBe(5)
wrapper.find("StyledTab").forEach((option, index) => {
expect(option.text()).toBe(`Tab ${index}`)
tabs.forEach((tab, index) => {
expect(tab).toHaveTextContent(`Tab ${index}`)
})
})

it("can be disabled", () => {
const wrapper = mount(<Tabs {...getProps({ widgetsDisabled: true })} />)
render(<Tabs {...getProps({ widgetsDisabled: true })} />)
const tabs = screen.getAllByRole("tab")

wrapper.find("StyledTab").forEach((option, index) => {
tabs.forEach((_, index) => {
// the selected tab does not have the disabled prop as true in baseweb
expect(option.prop("disabled")).toBe(index !== 0)
if (index == 0) {
return
}
expect(tabs[index]).toBeDisabled()
})
})
})
29 changes: 23 additions & 6 deletions frontend/lib/src/components/elements/Tabs/Tabs.tsx
Expand Up @@ -20,6 +20,7 @@ import { Tabs as UITabs, Tab as UITab } from "baseui/tabs-motion"

import { BlockNode, AppNode } from "@streamlit/lib/src/AppNode"
import { BlockPropsWithoutWidth } from "@streamlit/lib/src/components/core/Block"
import { isElementStale } from "@streamlit/lib/src/components/core/Block/utils"
import StreamlitMarkdown from "@streamlit/lib/src/components/shared/StreamlitMarkdown"

import { StyledTabContainer } from "./styled-components"
Expand All @@ -32,7 +33,7 @@ export interface TabProps extends BlockPropsWithoutWidth {
}

function Tabs(props: TabProps): ReactElement {
const { widgetsDisabled, node, isStale } = props
const { widgetsDisabled, node, isStale, scriptRunState, scriptRunId } = props

let allTabLabels: string[] = []
const [activeTabKey, setActiveTabKey] = useState<React.Key>(0)
Expand All @@ -51,6 +52,7 @@ function Tabs(props: TabProps): ReactElement {
const newTabKey = allTabLabels.indexOf(activeTabName)
if (newTabKey === -1) {
setActiveTabKey(0)
setActiveTabName(allTabLabels[0])
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [allTabLabels])
Expand All @@ -65,8 +67,10 @@ function Tabs(props: TabProps): ReactElement {
const newTabKey = allTabLabels.indexOf(activeTabName)
if (newTabKey !== -1) {
setActiveTabKey(newTabKey)
setActiveTabName(allTabLabels[newTabKey])
} else {
setActiveTabKey(0)
setActiveTabName(allTabLabels[0])
}

// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -134,8 +138,19 @@ function Tabs(props: TabProps): ReactElement {
// Reset available tab labels when rerendering
if (index === 0) allTabLabels = []

// If the tab is stale, disable it
const isStaleTab = isElementStale(
appNode,
scriptRunState,
scriptRunId
)
const disabled = widgetsDisabled || isStaleTab

// Ensure stale tab's elements are also marked stale/disabled
const childProps = {
...props,
isStale: isStale || isStaleTab,
widgetsDisabled: disabled,
node: appNode as BlockNode,
}
let nodeLabel = index.toString()
Expand All @@ -144,7 +159,8 @@ function Tabs(props: TabProps): ReactElement {
}
allTabLabels[index] = nodeLabel

const isSelected = activeTabKey.toString() === index.toString()
const isSelected =
activeTabKey.toString() === index.toString() && !isStaleTab
const isLast = index === node.children.length - 1

return (
Expand All @@ -157,6 +173,7 @@ function Tabs(props: TabProps): ReactElement {
/>
}
key={index}
disabled={disabled}
overrides={{
TabPanel: {
style: () => ({
Expand All @@ -176,25 +193,25 @@ function Tabs(props: TabProps): ReactElement {
paddingBottom: theme.spacing.none,
fontSize: theme.fontSizes.sm,
background: "transparent",
color: widgetsDisabled
color: disabled
? theme.colors.fadedText40
: theme.colors.bodyText,
":focus": {
outline: "none",
color: widgetsDisabled
color: disabled
? theme.colors.fadedText40
: theme.colors.primary,
background: "none",
},
":hover": {
color: widgetsDisabled
color: disabled
? theme.colors.fadedText40
: theme.colors.primary,
background: "none",
},
...(isSelected
? {
color: widgetsDisabled
color: disabled
? theme.colors.fadedText40
: theme.colors.primary,
}
Expand Down