UI Refactor [AARD-1903]

[PepperLola]

UI Refactor

AARD-1903

Refactored UI to improve the process of creating and using modals and panels. Rather than registering each possible UI element and then opening them with hardcoded IDs, the UI element itself is now passed into the relevant open* functions. In a tsx file, this looks something like openPanel(<ConfigurePanel />) but in regular .ts files it would look like openPanel(React.createElement(ConfigurePanel)).

Panels vs Modals

There are two types of screens: Panels and Modals. I decided to generalize these as UI screens. There can only ever be one modal open at a time, and the modals demand attention from the user, meaning they block interaction to everything else behind them until they're dismissed. There can be any number of panels open and are draggable, so they're intended to be passive and able to remain open on the side while using Synthesis. For example, the main menu is a modal because we need the user to choose a mode before we can load a scene. The configuration panel, on the other hand, doesn't require the attention of the user so it can be made a panel. It's my personal opinion that any screen that doesn't necessarily require the attention of the user should be made a panel, as a modal can disrupt a user's focus.

The bulk of the UI management happens in UIProvider. The screen type looks as follows:

interface UIScreen<T> {
    id: string
    parent?: UIScreen<unknown>
    content: ReactElement
    props: ModalProps | PanelProps
    onClose: UICallback<[CloseType], void>
    onCancel: UICallback<[], void>
    onAccept: UICallback<[T], void>
    onBeforeAccept: UICallback<[], T>
}

The ids are automatically generated when opening a screen and returned from the open function. They're required for closing a panel, which means you can only close a screen that you open (or if the code that opened the screen passes the id down elsewhere). This was a byproduct of the way I designed the opening (i.e. passing in a react component rather than an id so you don't have to register screens beforehand) but I think also makes sense as a requirement, as our previous code frequently closed panels in random places in a way that made the flow difficult to follow.

The parent is optional, but is a screen object referencing the screen that opened the current one, and comes from an optional argument in the open functions specifying the parent. This allows a screen to close its parent, as it has direct access to the id.

The content is just a ReactElement and is generally going to be a functional component that we define. This is what is rendered on the screen.

The props are either Modal- or Panel-specific properties. Each Modal and Panel interface that extends UIScreen<T> overwrite the props field with one of the relevant type. The properties interfaces are as follows:

interface UIScreenProps {
    configured: boolean
    title?: string
    hideCancel?: boolean
    hideAccept?: boolean
    cancelText?: string
    acceptText?: string
}

interface ModalProps extends UIScreenProps {
    // required for PanelProps to not satisfy ModalProps
    type: "modal"
    allowClickAway?: boolean
}

interface PanelProps extends UIScreenProps {
    type: "panel"
    position: PanelPosition
}

The configured property can be ignored, and is only used internally to denote whether the screen has been configured; the screen will only be rendered when once it has been configured. The title will render at the top of the screen. The hideCancel and hideAccept props will disable rendering of the relevant buttons when true, and cancelText and acceptText will change the text shown in the buttons. Each modal has a clickaway listener that will close it if the user clicks the background, but this behavior can be disabled by setting the allowClickAway property to false. As panels can exist alongside each other and can be moved, they also have configurable opening positions, which can be configured by setting the position property.

As the ModalProps interface only contains one optional field, there's nothing stopping a PanelProps instance from satisfying ModalProps, which is why the type fields were added. They can also be ignored and only serve the purpose of allowing the enforcement of the right properties type for each type of screen.

Callbacks

I created a UICallback object that can contain a default function and a user-defined function (or any combination of the two, as they are both optional). The user-defined function always runs first when defined, and the return value of the entire callback will always be that of the user-defined when it's not undefined. The user-defined functions are passed into the screen open functions, and the default functions are provided when calling configureScreen from within a screen element. Each screen has the following callbacks:

interface UIScreenCallbacks<T> {
    onClose?: (closeType: CloseType) => void
    onCancel?: () => void
    onBeforeAccept?: () => T
    onAccept?: (arg: T) => void
}

onClose is always called before any other callback when a screen is closed. The callback has a parameter of the CloseType, which can either be Accept, Cancel, or Overwrite. They are pretty self-explanatory - Accept and Cancel depend on the button that was pressed, and Overwrite is used when one screen takes the place of another. When a modal is opened, any previously open modal will also be closed and its onClose function will be called with the Overwrite close type. onCancel is called when the cancel button is clicked (or whenever a function is called to close a screen with the intent of canceling its purpose). onBeforeAccept is called, as the name suggests, before onAccept. It is only configurable from within a screen and can thus only contain a default implementation (passed in when calling configureScreen). It can return a value of type T (defined when creating a screen) that is then passed as an argument to onAccept, which can only contain a user-defined value configured in the open function for the screen.

UIProvider and UIRenderer

[UIProvider](https://github.com/Autodesk/synthesis/blob/jwrigh/1903/ui-refactor-tmp/fission/src/ui/UIProvider.tsx) exposes the current modal and panels, along with all the relevant ui-related functions:

type UIContextProps = {
    modal?: Modal<unknown>
    panels: Panel<unknown>[]
    openModal: OpenModalFn
    openPanel: OpenPanelFn
    closeModal: CloseModalFn
    closePanel: ClosePanelFn
    addToast: AddToastFn
    configureScreen: ConfigureScreenFn
}

These can be imported by calling the useUIContext hook from within a React component (e.g. within a panel or modal).

UIRenderer just renders the current modal and panels, passing through the props and parent.

Opening UI Screens

UI Screens can be opened using their relevant open functions, with the following type signatures:

type OpenModalFn = <T>(
    contents: ReactElement,
    parent?: UIScreen<T>,
    props?: Omit<ModalProps, "type" | "configured">
) => string
type OpenPanelFn = <T>(
    contents: ReactElement,
    parent?: UIScreen<T>,
    props?: Omit<PanelProps, "type" | "configured">
) => string

To open a screen, you need to call the relevant open function and pass in a ReactElement. You can optionally pass in a parent (which is where the parent property from before comes from) and properties you want to configure (which is where the user-defined callbacks come from). The functions will return the randomly-generated id for the new screen.

Closing UI Screens

To close a screen, you need to first have the id of the screen you want to close, which will have previously been returned from the function that opened that screen. To close the screen, you simply call the relevant close function. As there can only ever be one modal open at once, the closeModal function only requires a close type to be passed in, which was defined above to be one of Accept, Cancel, or Overwrite. The closePanel function requires the id of the panel to close along with the close type.

Rendering Toasts

I installed the notistack package to add a notification stack to the bottom-left corner of the screen like we had previously. UIProvider exposes an addToast function that takes in a type (from notistack) along with any number of string arguments beyond that, which will be joined by line breaks and rendered in the notification.

Global Functions

The GlobalUIControls file exports globalAddToast, globalOpenPanel and globalOpenModal which are all set to the respective functions from UIProvider. This is so that non-React-component code can perform any of these actions. This is especially important for adding toasts, as there are many places we would want to provide feedback to the user (e.g. in the event of an error) from code that isn't in a React component.

Creating a UI Screen

Creating a new screen is as simple as creating a new file with a functional component in the correct directory (i.e. src/ui/modals/ for modals, src/ui/panels/ for panels) and exporting it. The type of the component must be either PanelImplProps<T> or ModalImplProps<T>, where T is the type of the data to be returned from onBeforeAccept and passed into onAccept.

Those types allow the screens to access their object (Modal or Panel, the interface introduced earlier with the id, parent, etc.) along with that of their parent as props.

Each screen must also be configured, which must be done via the configureScreen function exposed by the UI context within UIProvider. This is necessary for all panels, even if you don't plan to override any properties or callbacks. If this isn't done, the UI renderer won't consider the screen to have been configured and as such will not render it.

The type signature for the configure function is a bit difficult to understand at first:

type ConfigureScreenFn = <T extends UIScreen<any>>(
    screen: T,
    props: T extends Panel<infer _> ? Partial<Omit<PanelProps, "configured">> : Partial<Omit<ModalProps, "configured">>,
    callbacks: T extends Modal<infer S>
        ? Omit<Partial<UIScreenCallbacks<S>>, "onAccept">
        : T extends Panel<infer S>
          ? Omit<Partial<UIScreenCallbacks<S>>, "onAccept">
          : never
) => void

It essentially takes either a panel or a modal as its first argument and, depending on which is provided, will require the props for that type to be passed as the second argument (guarding against configuring a modal with panel props). The callbacks provided here to the callbacks argument are then set as the default implementation for the callbacks (as opposed to the user-defined implementation; discussed above).

The APS management modal is a good, small example of creating a modal:

import { Stack } from "@mui/material"
import type React from "react"
import { useEffect, useState } from "react"
import { HiUser } from "react-icons/hi"
import APS from "@/aps/APS"
import type { ModalImplProps } from "@/ui/components/Modal"
import { useUIContext } from "../helpers/UIProviderHelpers"

const APSManagementModal: React.FC<ModalImplProps<void>> = ({ modal }) => {
    const { configureScreen } = useUIContext()
    const [userInfo, _] = useState(APS.userInfo)
    useEffect(() => {
        const onBeforeAccept = () => {
            APS.logout()
        }

        configureScreen(modal!, { title: userInfo?.name ?? "Not signed in", acceptText: "Logout" }, { onBeforeAccept })
    }, [modal, userInfo?.name])

    return (
        <Stack spacing={10} direction="row">
            {userInfo?.picture ? (
                <img alt={userInfo?.name} src={userInfo?.picture} className="h-10 rounded-full" />
            ) : (
                <HiUser />
            )}
        </Stack>
    )
}

export default APSManagementModal

We can see that it imported the configureScreen function using the useUIContext hook. This is also how to import the openModal, closeModal, openPanel, and closePanel functions.

It also called configureScreen within a useEffect, which is the preferred way of configuring a screen. Even if no configuration were necessary, it would still need to call configureScreen with empty property objects, as follows:

configureScreen(modal!, {}, {})

This modal could then be opened in another component like so:

// first import the openModal function
const { openModal } = useUIContext()

// then open the modal
openModal(<APSManagementModal />)

or from a regular TypeScript file, where you can't create a JSX/TSX element:

// first import React and the globalOpenModal function
import React from "react"
import { globalOpenModal } from "@/ui/components/GlobalUIControls"

// then open the modal
globalOpenModal(React.createElement(APSManagementModal))

MUI

I migrated (almost?) everything to use MUI Material UI instead of our own built-in components. Some more complex components were kept but internally use MUI components (e.g. SelectMenu), but some were discarded entirely (e.g. removed Dropdown in favor of MUI's Select).

The most important components going forward are Stack (flex box), Box (general container), Select (dropdown menu), Button (button), and TextField (text input field). A full list of MUI components can be found here.

---

Before merging, ensure the following criteria are met:

• All acceptance criteria outlined in the ticket are met.
• Necessary test cases have been added and updated.
• A feature toggle or safe disable path has been added (if applicable).
• User-facing polish:
  • Ask: "Is this ready-looking?"
• Cross-linking between Jira and GitHub:
  • PR links to the relevant Jira issue.
  • Jira ticket has a comment referencing this PR.

[AlexD717]

If you have a robot and a field spawned in, and you right click on the field and press configure the app crashes.

[Dhruv-0-Arora]

Graphics settings panel should be more descriptive. I think its better to just make this another PR though and get the UI Refactor merged.

This is what graphics settings looks like on dev

[I realized that its not to do with th graphics settings panel but how the sliders are formatted. The value of the slider is not displayed which, in my opinion, would be useful be to add]

[Dhruv-0-Arora]

Potential feature to be added in the future:

Save the positioning of panels in preferences so that they always open in the position set by the user.

[rutmanz]

I think all of the core functionality seems good, and it's just style things remaining

[ryanzhangofficial]

If the criteria for merging is no crashing and retaining functionality, I think we're good to go.

[PepperLola]

Graphics settings panel should be more descriptive. I think its better to just make this another PR though and get the UI Refactor merged.

This is what graphics settings looks like on dev

[I realized that its not to do with th graphics settings panel but how the sliders are formatted. The value of the slider is not displayed which, in my opinion, would be useful be to add]

These are style changes that are not within the scope of this PR

[Dhruv-0-Arora]

This is a work of ART

[AlexD717]

Lets get this in!

As a different ticket, we should consider removing a lot of the console.log statements, as it currently clutters up the console.

[BrandonPacewic]

@PepperLola I'll let you be the final review and merge for this PR. From what we discussed yesterday in standup everyone is ready to get this in and begin working on it. Plus we can finally merge everything else from the backlog.

All issues were fixed