Skip to content

Commit

Permalink
Merge anchor headers feature branch into develop (#2983)
Browse files Browse the repository at this point in the history
* Add anchors to Markdown headers (#2513)

* save progress

* save work

* remove junk code

* fix lint errors

* clean up code a bit

* fix incorrect type

* remove debug code

* fix heading numbers

* make requested changes and clean up HeadingWithAnchor

* only scroll once

* fix issues

* fix jslint

* clean things up

* handle edge case

* missing a void

* fix failing test

* add e2e tests

* add e2e tests

* clean up code by using _.once()

* fix lint

* fix dynamic scrolling

* fix typo

* make requested changes

* run formatting and remove unused import

* remove another unused import

* make requested changes

* add nice link icon to left of headers

* make requested design changes (#2726)

* Fix anchor styling to work with dark mode (#3035)

* fix anchor styling

* Revert "fix anchor styling"

This reverts commit cb5d7fd.

* fix failing cypress tests

* fix styling

* Add S4A communication for anchor headers (#2982)

* add communication with s4a

* fix bug

* save work

* save work

* remove console

* add tests

* remove useless import

* fix failing tests

* Remove Markdown anchors from sidebar (#3059)

* remove markdown anchors from sidebar

* fix test

* add tests

* Fix failing Cypress test for Markdown Anchors (#3077)

* see if this fixes things

* fix things

* fix

* fix scrolling

* add spacer

* save work

* remove unneeded changes
  • Loading branch information
bh-streamlit committed Apr 6, 2021
1 parent 3477e7b commit 43bab9d
Show file tree
Hide file tree
Showing 22 changed files with 484 additions and 47 deletions.
1 change: 1 addition & 0 deletions e2e/scripts/st_header.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
import streamlit as st

st.header("This header is awesome!")
st.header("This header is awesome too!", anchor="awesome-header")
4 changes: 4 additions & 0 deletions e2e/scripts/st_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
"""
)

st.markdown("# Some header 1")
st.markdown("## Some header 2")
st.markdown("### Some header 3")

st.markdown(
"""
| Col1 | Col2 |
Expand Down
1 change: 1 addition & 0 deletions e2e/scripts/st_subheader.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
import streamlit as st

st.subheader("This subheader is awesome!")
st.subheader("This subheader is awesome too!", anchor="awesome-subheader")
1 change: 1 addition & 0 deletions e2e/scripts/st_title.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
import streamlit as st

st.title("This title is awesome!")
st.title("This title is awesome too!", anchor="awesome-title")
19 changes: 15 additions & 4 deletions e2e/specs/st_header.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ describe("st.header", () => {
cy.visit("http://localhost:3000/");
});

it("displays correct number of elements", () => {
cy.get(".element-container .stMarkdown h2").should("have.length", 2);
});

it("displays a header", () => {
cy.get(".element-container .stMarkdown h2").should(
"contain",
"This header is awesome!"
);
cy.get(".element-container .stMarkdown h2").then(els => {
expect(els[0].textContent).to.eq("This header is awesome!");
expect(els[1].textContent).to.eq("This header is awesome too!");
});
});

it("displays headers with anchors", () => {
cy.get(".element-container .stMarkdown h2").then(els => {
cy.wrap(els[0]).should("have.attr", "id", "this-header-is-awesome");
cy.wrap(els[1]).should("have.attr", "id", "awesome-header");
});
});
});
28 changes: 25 additions & 3 deletions e2e/specs/st_markdown.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@
describe("st.markdown", () => {
before(() => {
cy.visit("http://localhost:3000/");

// Make the ribbon decoration line disappear
cy.get("[data-testid='stDecoration']").invoke("css", "display", "none");
});

it("displays correct number of elements", () => {
cy.get(".element-container .stMarkdown").should("have.length", 12);
});

it("displays markdown", () => {
cy.get(".element-container .stMarkdown").should("have.length", 9);
cy.get(".element-container .stMarkdown").then(els => {
expect(els[0].textContent).to.eq("This markdown is awesome! 😎");
expect(els[1].textContent).to.eq("This <b>HTML tag</b> is escaped!");
Expand All @@ -33,18 +39,34 @@ describe("st.markdown", () => {
expect(els[7].textContent).to.eq(
"ax2+bx+c=0ax^2 + bx + c = 0ax2+bx+c=0"
);
expect(els[8].textContent).to.eq("Col1Col2SomeData");
expect(els[8].textContent).to.eq("Some header 1");
expect(els[9].textContent).to.eq("Some header 2");
expect(els[10].textContent).to.eq("Some header 3");
expect(els[11].textContent).to.eq("Col1Col2SomeData");

cy.wrap(els[3])
.find("a")
.should("not.exist");

cy.wrap(els[4])
.find("a")
.should("have.attr", "href");
});
});

it("displays headers with anchors", () => {
cy.get(".element-container .stMarkdown").then(els => {
cy.wrap(els[8])
.find("h1")
.should("have.attr", "id", "some-header-1");
cy.wrap(els[9])
.find("h2")
.should("have.attr", "id", "some-header-2");
cy.wrap(els[10])
.find("h3")
.should("have.attr", "id", "some-header-3");
});
});

it("has consistent st.markdown visuals", () => {
cy.get(".element-container .stMarkdown").each((el, i) => {
// The 6th st.markdown element is an empty one, so cypress gets confused
Expand Down
19 changes: 15 additions & 4 deletions e2e/specs/st_subheader.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ describe("st.subheader", () => {
cy.visit("http://localhost:3000/");
});

it("displays correct number of elements", () => {
cy.get(".element-container .stMarkdown h3").should("have.length", 2);
});

it("displays a subheader", () => {
cy.get(".element-container .stMarkdown h3").should(
"contain",
"This subheader is awesome!"
);
cy.get(".element-container .stMarkdown h3").then(els => {
expect(els[0].textContent).to.eq("This subheader is awesome!");
expect(els[1].textContent).to.eq("This subheader is awesome too!");
});
});

it("displays subheaders with anchors", () => {
cy.get(".element-container .stMarkdown h3").then(els => {
cy.wrap(els[0]).should("have.attr", "id", "this-subheader-is-awesome");
cy.wrap(els[1]).should("have.attr", "id", "awesome-subheader");
});
});
});
19 changes: 15 additions & 4 deletions e2e/specs/st_title.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ describe("st.title", () => {
cy.visit("http://localhost:3000/");
});

it("displays correct number of elements", () => {
cy.get(".element-container .stMarkdown h1").should("have.length", 2);
});

it("displays a title", () => {
cy.get(".element-container .stMarkdown h1").should(
"contain",
"This title is awesome!"
);
cy.get(".element-container .stMarkdown h1").then(els => {
expect(els[0].textContent).to.eq("This title is awesome!");
expect(els[1].textContent).to.eq("This title is awesome too!");
});
});

it("displays title with anchors", () => {
cy.get(".element-container .stMarkdown h1").then(els => {
cy.wrap(els[0]).should("have.attr", "id", "this-title-is-awesome");
cy.wrap(els[1]).should("have.attr", "id", "awesome-title");
});
});
});
23 changes: 23 additions & 0 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
SessionState,
Config,
} from "src/autogen/proto"
import { without, concat } from "lodash"

import { RERUN_PROMPT_MODAL_DIALOG } from "src/lib/baseconsts"
import { SessionInfo } from "src/lib/SessionInfo"
Expand Down Expand Up @@ -120,6 +121,7 @@ interface State {
initialSidebarState: PageConfig.SidebarState
allowRunOnSave: boolean
deployParams?: IDeployParams | null
reportFinishedHandlers: (() => void)[]
developerMode: boolean
themeHash?: string
}
Expand Down Expand Up @@ -178,6 +180,7 @@ export class App extends PureComponent<Props, State> {
initialSidebarState: PageConfig.SidebarState.AUTO,
allowRunOnSave: true,
deployParams: null,
reportFinishedHandlers: [],
// A hack for now to get theming through. Product to think through how
// developer mode should be designed in the long term.
developerMode: window.location.host.includes("localhost"),
Expand Down Expand Up @@ -647,6 +650,12 @@ export class App extends PureComponent<Props, State> {
*/
handleReportFinished(status: ForwardMsg.ReportFinishedStatus): void {
if (status === ForwardMsg.ReportFinishedStatus.FINISHED_SUCCESSFULLY) {
// Notify any subscribers of this event (and do it on the next cycle of
// the event loop)
window.setTimeout(() => {
this.state.reportFinishedHandlers.map(handler => handler())
}, 0)

// Clear any stale elements left over from the previous run.
// (We don't do this if our script had a compilation error and didn't
// finish successfully.)
Expand Down Expand Up @@ -1000,6 +1009,18 @@ export class App extends PureComponent<Props, State> {
this.setState({ isFullScreen })
}

addReportFinishedHandler = (func: () => void): void => {
this.setState({
reportFinishedHandlers: concat(this.state.reportFinishedHandlers, func),
})
}

removeReportFinishedHandler = (func: () => void): void => {
this.setState({
reportFinishedHandlers: without(this.state.reportFinishedHandlers, func),
})
}

render(): JSX.Element {
const {
allowRunOnSave,
Expand Down Expand Up @@ -1040,6 +1061,8 @@ export class App extends PureComponent<Props, State> {
embedded: isEmbeddedInIFrame(),
isFullScreen,
setFullScreen: this.handleFullScreen,
addReportFinishedHandler: this.addReportFinishedHandler,
removeReportFinishedHandler: this.removeReportFinishedHandler,
activeTheme: this.props.theme.activeTheme,
availableThemes: this.props.theme.availableThemes,
setTheme: this.props.theme.setTheme,
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/components/core/PageLayoutContext/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export interface Props {
embedded: boolean
isFullScreen: boolean
setFullScreen: (value: boolean) => void
addReportFinishedHandler: (func: () => void) => void
removeReportFinishedHandler: (func: () => void) => void
activeTheme: ThemeConfig
setTheme: (theme: ThemeConfig) => void
availableThemes: ThemeConfig[]
Expand All @@ -39,6 +41,8 @@ export default React.createContext<Props>({
embedded: false,
isFullScreen: false,
setFullScreen: (value: boolean) => {},
addReportFinishedHandler: (func: () => void) => {},
removeReportFinishedHandler: (func: () => void) => {},
activeTheme: baseTheme,
setTheme: (theme: ThemeConfig) => {},
availableThemes: [],
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/components/core/ReportView/ReportView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ReportRunState } from "src/lib/ReportRunState"
import { WidgetStateManager } from "src/lib/WidgetStateManager"
import { FileUploadClient } from "src/lib/FileUploadClient"
import { ComponentRegistry } from "src/components/widgets/CustomComponent"
import { sendS4AMessage } from "src/hocs/withS4ACommunication/withS4ACommunication"

import PageLayoutContext from "src/components/core/PageLayoutContext"
import { BlockNode, ReportRoot } from "src/lib/ReportNode"
Expand Down Expand Up @@ -75,6 +76,17 @@ function ReportView(props: ReportViewProps): ReactElement {
componentRegistry,
} = props

React.useEffect(() => {
const listener = (): void => {
sendS4AMessage({
type: "UPDATE_HASH",
hash: window.location.hash,
})
}
window.addEventListener("hashchange", listener, false)
return () => window.removeEventListener("hashchange", listener, false)
}, [])

const { wideMode, initialSidebarState, embedded } = React.useContext(
PageLayoutContext
)
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/components/core/Sidebar/IsSidebarContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import React from "react"

export default React.createContext(false)
11 changes: 4 additions & 7 deletions frontend/src/components/core/Sidebar/Sidebar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,14 @@
*/

import React from "react"
import { ShallowWrapper } from "enzyme"
import { shallow } from "src/lib/test_util"
import { ReactWrapper } from "enzyme"
import { mount } from "src/lib/test_util"
import { PageConfig } from "src/autogen/proto"

import Sidebar, { SidebarProps } from "./Sidebar"

function renderSideBar(props: Partial<SidebarProps>): ShallowWrapper {
// Diving twice to render the WithTheme
return shallow(<Sidebar {...props} />)
.dive()
.dive()
function renderSideBar(props: Partial<SidebarProps>): ReactWrapper {
return mount(<Sidebar {...props} />)
}

describe("Sidebar Component", () => {
Expand Down
11 changes: 10 additions & 1 deletion frontend/src/components/core/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
StyledSidebarCollapsedControl,
StyledSidebarContent,
} from "./styled-components"
import IsSidebarContext from "./IsSidebarContext"

export interface SidebarProps {
children?: ReactElement
Expand Down Expand Up @@ -171,4 +172,12 @@ class Sidebar extends PureComponent<SidebarProps, State> {
}
}

export default withTheme(Sidebar)
function SidebarWithProvider(props: SidebarProps): ReactElement {
return (
<IsSidebarContext.Provider value={true}>
<Sidebar {...props} />
</IsSidebarContext.Provider>
)
}

export default withTheme(SidebarWithProvider)
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@

import React, { ReactElement } from "react"
import ReactMarkdown from "react-markdown"
import { mount } from "enzyme"
import { mount } from "src/lib/test_util"
import IsSidebarContext from "src/components/core/Sidebar/IsSidebarContext"

import {
import StreamlitMarkdown, {
linkWithTargetBlank,
linkReferenceHasParens,
createAnchorFromText,
} from "./StreamlitMarkdown"

import { StyledLinkIconContainer } from "./styled-components"

// Fixture Generator
const getMarkdownElement = (body: string): ReactElement => {
const renderers = {
Expand All @@ -33,6 +37,20 @@ const getMarkdownElement = (body: string): ReactElement => {
return <ReactMarkdown source={body} renderers={renderers} />
}

describe("createAnchorFromText", () => {
it("generates slugs correctly", () => {
const cases = [
["some header", "some-header"],
["some -24$35-9824 header", "some-24-35-9824-header"],
["blah___blah___blah", "blah-blah-blah"],
]

cases.forEach(([s, want]) => {
expect(createAnchorFromText(s)).toEqual(want)
})
})
})

describe("linkReference", () => {
it("renders a link with _blank target", () => {
const body = "Some random URL like [Streamlit](https://streamlit.io/)"
Expand Down Expand Up @@ -83,3 +101,25 @@ describe("linkReference", () => {
expect(wrapper.find("a").exists()).toBe(false)
})
})

describe("StreamlitMarkdown", () => {
it("renders header anchors when isSidebar is false", () => {
const source = "# header"
const wrapper = mount(
<IsSidebarContext.Provider value={false}>
<StreamlitMarkdown source={source} allowHTML={false} />
</IsSidebarContext.Provider>
)
expect(wrapper.find(StyledLinkIconContainer).exists()).toBeTruthy()
})

it("doesn't render header anchors when isSidebar is true", () => {
const source = "# header"
const wrapper = mount(
<IsSidebarContext.Provider value={true}>
<StreamlitMarkdown source={source} allowHTML={false} />
</IsSidebarContext.Provider>
)
expect(wrapper.find(StyledLinkIconContainer).exists()).toBeFalsy()
})
})

0 comments on commit 43bab9d

Please sign in to comment.