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

Markdown support for radio button labels #7018

Merged
merged 8 commits into from
Jul 25, 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
16 changes: 14 additions & 2 deletions e2e/scripts/st_radio.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@
from tests.streamlit import pyspark_mocks

options = ("female", "male")
markdown_options = (
"**bold text**",
"*italics text*",
"~strikethrough text~",
"shortcode: :blush:",
# link should not work in radio options
"[link text](www.example.com)",
"`code text`",
":red[red] :blue[blue] :green[green] :violet[violet] :orange[orange]",
)
i1 = st.radio("radio 1", options, 1)
st.write("value 1:", i1)

Expand All @@ -43,14 +53,16 @@
i8 = st.radio("radio 8", options, label_visibility="collapsed")
st.write("value 8:", i8)

i9 = st.radio("radio 9", markdown_options)
st.write("value 9:", i9)

if runtime.exists():

def on_change():
st.session_state.radio_changed = True

st.radio("radio 9", options, 1, key="radio9", on_change=on_change)
st.write("value 9:", st.session_state.radio9)
st.radio("radio 10", options, 1, key="radio10", on_change=on_change)
st.write("value 10:", st.session_state.radio10)
st.write("radio changed:", "radio_changed" in st.session_state)

st.radio("PySpark radio", pyspark_mocks.DataFrame()) # type: ignore
13 changes: 8 additions & 5 deletions e2e/specs/st_radio.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe("st.radio", () => {
});

it("shows widget correctly", () => {
cy.get(".stRadio").should("have.length", 10);
cy.get(".stRadio").should("have.length", 11);

cy.get(".stRadio").each((el, idx) => {
return cy.wrap(el).matchThemedSnapshots("radio" + idx);
Expand Down Expand Up @@ -95,7 +95,8 @@ describe("st.radio", () => {
"value 6: female" +
"value 7: female" +
"value 8: female" +
"value 9: male" +
"value 9: bold text" +
"value 10: male" +
"radio changed: False"
);
});
Expand Down Expand Up @@ -139,13 +140,14 @@ describe("st.radio", () => {
"value 6: male" +
"value 7: male" +
"value 8: male" +
"value 9: male" +
"value 9: red blue green violet orange" +
"value 10: male" +
"radio changed: False"
);
});

it("calls callback if one is registered", () => {
cy.getIndexed(".stRadio", 8).then(el => {
cy.getIndexed(".stRadio", 9).then(el => {
return cy
.wrap(el)
.find("input")
Expand All @@ -163,7 +165,8 @@ describe("st.radio", () => {
"value 6: female" +
"value 7: female" +
"value 8: female" +
"value 9: female" +
"value 9: bold text" +
"value 10: female" +
"radio changed: True"
);
});
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
115 changes: 57 additions & 58 deletions frontend/lib/src/components/shared/Radio/Radio.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
*/

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

import { Radio as UIRadio, RadioGroup, ALIGN } from "baseui/radio"
import { LabelVisibilityOptions } from "@streamlit/lib/src/util/utils"
import { mockTheme } from "@streamlit/lib/src/mocks/mockTheme"
import Radio, { Props } from "./Radio"
Expand All @@ -37,112 +38,110 @@ const getProps = (props: Partial<Props> = {}): Props => ({
describe("Radio widget", () => {
it("renders without crashing", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
render(<Radio {...props} />)
const radioGroup = screen.getByRole("radiogroup")
const radioOptions = screen.getAllByRole("radio")

expect(wrapper.find(RadioGroup).length).toBe(1)
expect(wrapper.find(UIRadio).length).toBe(3)
expect(radioGroup).toBeInTheDocument()
expect(radioOptions).toHaveLength(3)
})

it("renders without crashing if no label is provided", () => {
const props = getProps({ label: undefined })
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find(RadioGroup).length).toBe(1)
expect(wrapper.find(UIRadio).length).toBe(3)
render(<Radio {...props} />)
const widgetLabel = screen.queryByText("Label")
const radioOptions = screen.getByRole("radiogroup")

expect(widgetLabel).toBeNull()
expect(radioOptions).toBeInTheDocument()
})

it("pass labelVisibility prop to StyledWidgetLabel correctly when hidden", () => {
const props = getProps({
labelVisibility: LabelVisibilityOptions.Hidden,
})
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find("StyledWidgetLabel").prop("labelVisibility")).toEqual(
LabelVisibilityOptions.Hidden
)
render(<Radio {...props} />)

const widgetLabel = screen.getByText("Label")
expect(widgetLabel).toHaveStyle("visibility: hidden")
expect(widgetLabel).not.toBeVisible()
})

it("pass labelVisibility prop to StyledWidgetLabel correctly when collapsed", () => {
const props = getProps({
labelVisibility: LabelVisibilityOptions.Collapsed,
})
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find("StyledWidgetLabel").prop("labelVisibility")).toEqual(
LabelVisibilityOptions.Collapsed
)
render(<Radio {...props} />)
const widgetLabel = screen.getByText("Label")
expect(widgetLabel).not.toBeVisible()
})

it("has correct className and style", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
const wrappedDiv = wrapper.find("div").first()

const { className, style } = wrappedDiv.props()
// @ts-expect-error
const splittedClassName = className.split(" ")
render(<Radio {...props} />)
const radioElement = screen.getByTestId("stRadio")

expect(splittedClassName).toContain("row-widget")
expect(splittedClassName).toContain("stRadio")

// @ts-expect-error
expect(style.width).toBe(getProps().width)
expect(radioElement).toHaveClass("row-widget")
expect(radioElement).toHaveClass("stRadio")
expect(radioElement).toHaveStyle(`width: ${props.width}px`)
})

it("renders a label", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find("StyledWidgetLabel").text()).toBe(props.label)
render(<Radio {...props} />)
const widgetLabel = screen.queryByText(`${props.label}`)

expect(widgetLabel).toBeInTheDocument()
})

it("has a default value", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find(RadioGroup).prop("value")).toBe(props.value.toString())
render(<Radio {...props} />)
const radioOptions = screen.getAllByRole("radio")
expect(radioOptions).toHaveLength(3)

const checked = radioOptions[props.value]
expect(checked).toBeChecked()
})

it("can be disabled", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find(RadioGroup).prop("disabled")).toBe(props.disabled)
})
const props = getProps({ disabled: true })
render(<Radio {...props} />)
const radioOptions = screen.getAllByRole("radio")

it("can be horizontally aligned", () => {
const props = getProps({ horizontal: true })
const wrapper = mount(<Radio {...props} />)
expect(wrapper.find(RadioGroup).prop("align")).toBe(ALIGN.horizontal)
radioOptions.forEach(option => {
expect(option).toBeDisabled()
})
})

it("has the correct options", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
const options = wrapper.find(UIRadio)
render(<Radio {...props} />)

options.forEach((option, index) => {
expect(option.prop("value")).toBe(index.toString())
expect(option.prop("children")).toBe(props.options[index])
props.options.forEach(option => {
expect(screen.getByText(option)).toBeInTheDocument()
})
})

it("shows a message when there are no options to be shown", () => {
const props = getProps({ options: [] })
const wrapper = mount(<Radio {...props} />)
render(<Radio {...props} />)
const radioOptions = screen.getAllByRole("radio")
const noOptionLabel = screen.getByText("No options to select.")

expect(wrapper.find(UIRadio).length).toBe(1)
expect(wrapper.find(UIRadio).prop("children")).toBe(
"No options to select."
)
expect(radioOptions).toHaveLength(1)
expect(noOptionLabel).toBeInTheDocument()
})

it("handles value changes", () => {
const props = getProps()
const wrapper = mount(<Radio {...props} />)
render(<Radio {...props} />)
const radioOptions = screen.getAllByRole("radio")

const secondOption = radioOptions[1]

// @ts-expect-error
wrapper.find(RadioGroup).prop("onChange")({
target: {
value: "1",
},
} as React.ChangeEvent<HTMLInputElement>)
wrapper.update()
fireEvent.click(secondOption)

expect(wrapper.find(RadioGroup).prop("value")).toBe("1")
expect(secondOption).toBeChecked()
})
})
11 changes: 9 additions & 2 deletions frontend/lib/src/components/shared/Radio/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import TooltipIcon from "@streamlit/lib/src/components/shared/TooltipIcon"
import { LabelVisibilityOptions } from "@streamlit/lib/src/util/utils"
import { Placement } from "@streamlit/lib/src/components/shared/Tooltip"
import { EmotionTheme } from "@streamlit/lib/src/theme"
import StreamlitMarkdown from "@streamlit/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown"

export interface Props {
disabled: boolean
Expand Down Expand Up @@ -85,7 +86,7 @@ class Radio extends React.PureComponent<Props, State> {
}

return (
<div className="row-widget stRadio" style={style}>
<div className="row-widget stRadio" data-testid="stRadio" style={style}>
<WidgetLabel
label={label}
disabled={disabled}
Expand All @@ -103,6 +104,7 @@ class Radio extends React.PureComponent<Props, State> {
disabled={disabled}
align={horizontal ? ALIGN.horizontal : ALIGN.vertical}
aria-label={label}
data-testid="stRadioGroup"
>
{options.map((option: string, index: number) => (
<UIRadio
Expand Down Expand Up @@ -158,7 +160,12 @@ class Radio extends React.PureComponent<Props, State> {
},
}}
>
{option}
<StreamlitMarkdown
source={option}
allowHTML={false}
isLabel
isButton
/>
</UIRadio>
))}
</RadioGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,12 @@ describe("StreamlitMarkdown", () => {

it("doesn't render links when isButton is true", () => {
// Valid markdown further restricted with buttons to eliminate links
const source = "[text](www.example.com)"
const source = "[Link text](www.example.com)"
render(
<StreamlitMarkdown source={source} allowHTML={false} isLabel isButton />
)
const markdown = screen.getByText("text")
const tagName = markdown.nodeName.toLowerCase()
expect(tagName).not.toBe("a")
const tag = screen.getByText("Link text")
expect(tag instanceof HTMLAnchorElement).toBe(false)
})

it("renders smaller text sizing when isToast is true", () => {
Expand All @@ -266,6 +265,21 @@ describe("StreamlitMarkdown", () => {
expect(textTag).toHaveStyle("font-size: 14px")
})

it("renders regular text sizing when largerLabel is true", () => {
const source = "Here is some checkbox label text"
render(
<StreamlitMarkdown
source={source}
allowHTML={false}
isLabel
largerLabel
/>
)

const textTag = screen.getByText("Here is some checkbox label text")
expect(textTag).toHaveStyle("font-size: inherit")
})

it("colours text properly", () => {
const colorMapping = new Map([
["red", colors.red80],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ export interface Props {
isLabel?: boolean

/**
* Does not allow links
* Checkbox labels have larger font sizing
*/
isButton?: boolean
largerLabel?: boolean

/**
* Checkbox has larger label font sizing
* Does not allow links & has larger font sizing
*/
isCheckbox?: boolean
isButton?: boolean

/**
* Toast has smaller font sizing
Expand Down Expand Up @@ -233,7 +233,7 @@ export interface RenderedMarkdownProps {
isLabel?: boolean

/**
* Does not allow links
* Does not allow links & has larger font sizing
*/
isButton?: boolean
}
Expand Down Expand Up @@ -387,8 +387,8 @@ class StreamlitMarkdown extends PureComponent<Props> {
style,
isCaption,
isLabel,
largerLabel,
isButton,
isCheckbox,
isToast,
} = this.props
const isInSidebar = this.context
Expand All @@ -398,8 +398,8 @@ class StreamlitMarkdown extends PureComponent<Props> {
isCaption={Boolean(isCaption)}
isInSidebar={isInSidebar}
isLabel={isLabel}
largerLabel={largerLabel}
isButton={isButton}
isCheckbox={isCheckbox}
isToast={isToast}
style={style}
data-testid={isCaption ? "stCaptionContainer" : "stMarkdownContainer"}
Expand Down