Skip to content

Commit

Permalink
Migrate frontend navigation to App Navigation Abstraction (#8631)
Browse files Browse the repository at this point in the history
## Describe your changes

Combine all logic of the navigation (page url updates, mpa management),
and put it in a class (V1AppNavigation). We create a layer above it that
essentially pulls from the V1AppNavigation as an overengineering effort
to set up for AppNavigation V2

## Testing Plan

- Original tests should pass
- unit tests for AppNavigation.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
kmcgrady committed May 13, 2024
1 parent 34f8d7a commit 2d10e6a
Show file tree
Hide file tree
Showing 3 changed files with 497 additions and 69 deletions.
94 changes: 25 additions & 69 deletions frontend/app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
StreamlitEndpoints,
ensureError,
LibContext,
AppPage,
AutoRerun,
BackMsg,
Config,
Expand Down Expand Up @@ -124,6 +123,7 @@ import withScreencast, {
import "@streamlit/app/src/assets/css/theme.scss"
import { preserveEmbedQueryParams } from "@streamlit/lib/src/util/utils"
import { ThemeManager } from "./util/useThemeManager"
import { AppNavigation, MaybeStateUpdate } from "./util/AppNavigation"

export interface Props {
screenCast: ScreenCastHOC
Expand Down Expand Up @@ -233,6 +233,8 @@ export class App extends PureComponent<Props, State> {

private readonly embeddingId: string = generateUID()

private readonly appNavigation: AppNavigation

public constructor(props: Props) {
super(props)

Expand Down Expand Up @@ -360,6 +362,12 @@ export class App extends PureComponent<Props, State> {

this.pendingElementsTimerRunning = false
this.pendingElementsBuffer = this.state.elements
this.appNavigation = new AppNavigation(
this.hostCommunicationMgr,
this.metricsMgr,
this.maybeUpdatePageUrl,
this.onPageNotFound
)

window.streamlitDebug = {
clearForwardMsgCache: this.debugClearForwardMsgCache,
Expand Down Expand Up @@ -709,28 +717,11 @@ export class App extends PureComponent<Props, State> {
}

handlePageNotFound = (pageNotFound: PageNotFound): void => {
const { pageName } = pageNotFound

this.onPageNotFound(pageName)

const currentPageScriptHash = this.state.appPages[0]?.pageScriptHash || ""
this.setState({ currentPageScriptHash }, () => {
this.hostCommunicationMgr.sendMessageToHost({
type: "SET_CURRENT_PAGE_NAME",
currentPageName: "",
currentPageScriptHash,
})
})
this.maybeSetState(this.appNavigation.handlePageNotFound(pageNotFound))
}

handlePagesChanged = (pagesChangedMsg: PagesChanged): void => {
const { appPages } = pagesChangedMsg
this.setState({ appPages }, () => {
this.hostCommunicationMgr.sendMessageToHost({
type: "SET_APP_PAGES",
appPages,
})
})
this.maybeSetState(this.appNavigation.handlePagesChanged(pagesChangedMsg))
}

handlePageProfileMsg = (pageProfile: PageProfile): void => {
Expand Down Expand Up @@ -884,6 +875,14 @@ export class App extends PureComponent<Props, State> {
}
}

maybeSetState(stateUpdate: MaybeStateUpdate): void {
if (stateUpdate) {
const [newState, callback] = stateUpdate

this.setState(newState as State, callback)
}
}

/**
* Handler for ForwardMsg.newSession messages. This runs on each rerun
* @param newSessionProto a NewSession protobuf
Expand Down Expand Up @@ -913,18 +912,6 @@ export class App extends PureComponent<Props, State> {
pageScriptHash: newPageScriptHash,
} = newSessionProto

// mainPage must be a string as we're guaranteed at this point that
// newSessionProto.appPages is nonempty and has a truthy pageName.
// Otherwise, we'd either have no main script or a nameless main script,
// neither of which can happen.
const mainPage = newSessionProto.appPages[0] as AppPage
// We're similarly guaranteed that newPageName will be found / truthy
// here.
const newPageName = newSessionProto.appPages.find(
p => p.pageScriptHash === newPageScriptHash
)?.pageName as string
const viewingMainPage = newPageScriptHash === mainPage.pageScriptHash

if (!fragmentIdsThisRun.length) {
// This is a normal rerun, remove all the auto reruns intervals
this.state.autoReruns.forEach((value: NodeJS.Timer) => {
Expand All @@ -935,7 +922,6 @@ export class App extends PureComponent<Props, State> {
const config = newSessionProto.config as Config
const themeInput = newSessionProto.customTheme as CustomThemeConfig

this.maybeUpdatePageUrl(mainPage.pageName, newPageName, viewingMainPage)
this.processThemeInput(themeInput)
this.setState({
allowRunOnSave: config.allowRunOnSave,
Expand All @@ -946,33 +932,9 @@ export class App extends PureComponent<Props, State> {
// empty array.
fragmentIdsThisRun,
})
this.maybeSetState(this.appNavigation.handleNewSession(newSessionProto))

// We separate the state of the navigation as we intend to perform
// this work through an abstraction of multipage apps in advance
// of supporting v2.
this.setState(
{
hideSidebarNav: config.hideSidebarNav,
appPages: newSessionProto.appPages,
currentPageScriptHash: newPageScriptHash,
},
() => {
this.hostCommunicationMgr.sendMessageToHost({
type: "SET_APP_PAGES",
appPages: newSessionProto.appPages,
})

this.hostCommunicationMgr.sendMessageToHost({
type: "SET_CURRENT_PAGE_NAME",
currentPageName: viewingMainPage ? "" : newPageName,
currentPageScriptHash: newPageScriptHash,
})
}
)

// Set the title and favicon to their default values if we are not running
// a fragment.
document.title = `${newPageName} · Streamlit`
// Set the favicon to its default values
handleFavicon(
`${process.env.PUBLIC_URL}/favicon.png`,
this.hostCommunicationMgr.sendMessageToHost,
Expand All @@ -992,10 +954,7 @@ export class App extends PureComponent<Props, State> {
this.metricsMgr.setMetadata(this.state.deployedAppMetadata)
this.metricsMgr.setAppHash(newSessionHash)

this.metricsMgr.enqueue("updateReport", {
numPages: newSessionProto.appPages.length,
isMainPage: viewingMainPage,
})
this.appNavigation.sendMPAMetricsOnInitialization()

if (
appHash === newSessionHash &&
Expand Down Expand Up @@ -1031,12 +990,9 @@ export class App extends PureComponent<Props, State> {
* Handler called when the history state changes, e.g. `popstate` event.
*/
onHistoryChange = (): void => {
const targetAppPage =
this.state.appPages.find(appPage =>
// The page name is embedded at the end of the URL path, and if not, we are in the main page.
// See https://github.com/streamlit/streamlit/blob/1.19.0/frontend/src/App.tsx#L740
document.location.pathname.endsWith("/" + appPage.pageName)
) ?? this.state.appPages[0]
const targetAppPage = this.appNavigation.findPageByUrlPath(
document.location.pathname
)

// do not cause a rerun when an anchor is clicked and we aren't changing pages
const hasAnchor = document.location.toString().includes("#")
Expand Down

0 comments on commit 2d10e6a

Please sign in to comment.