Skip to content

Move deprecated AppUI functionality into mobile-ui-react. #75

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.

Sign up for GitHub

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

Draft
tcobbs-bentley wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Move deprecated AppUI functionality into mobile-ui-react. #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48bc75d
Move deprecated AppUI functionality into mobile-ui-react.
tcobbs-bentley Jul 16, 2024
fa04ec8
Merge branch 'main' into travis/appui-deprecations-local-copies
tcobbs-bentley Jul 16, 2024
7e30ace
Further AppUI deprecation fixes
tcobbs-bentley Jul 16, 2024
3b03df8
Merge branch 'main' into travis/appui-deprecations-local-copies
tcobbs-bentley Jul 17, 2024
852d7dc
Stop using SessionStateActionId.SetIModelConnection
tcobbs-bentley Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/mobile-ui-react/ActionSheetButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import { CommonProps, IconSpec } from "@itwin/core-react";
import { IconSpec } from "@itwin/core-react";
import { ActionSheetProps, presentActionSheet } from "@itwin/mobile-sdk-core";
import { NavigationButton } from "./NavigationPanel";
import { CommonProps } from "./MobileUi";

/**
* Properties for {@link ActionSheetButton}
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface ActionSheetButtonProps extends ActionSheetProps, CommonProps {
/** The icon to show on the {@link ActionSheetButton}, default is three vertical dots. */
iconSpec?: IconSpec;
Expand Down
15 changes: 8 additions & 7 deletions src/mobile-ui-react/BottomPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
import * as React from "react";
import classnames from "classnames";
import { BeUiEvent } from "@itwin/core-bentley";
import { CommonProps, getCssVariableAsNumber } from "@itwin/core-react";
import { Optional } from "@itwin/mobile-sdk-core";
import { makeRefHandler, MutableHtmlDivRefOrFunction, useBeUiEvent, useWindowEvent } from "./MobileUi";
import { getCssVariableAsNumber, Optional } from "@itwin/mobile-sdk-core";
import {
CommonProps,
makeRefHandler,
MutableHtmlDivRefOrFunction,
useBeUiEvent,
useWindowEvent,
} from "./MobileUi";
import { PanelHeader, PanelHeaderProps } from "./PanelHeader";
import { ResizablePanel, ResizablePanelProps } from "./ResizablePanel";
import "./BottomPanel.scss";
Expand Down Expand Up @@ -89,8 +94,6 @@ export function useBottomPanelTop() {
* Properties for the {@link BottomPanel} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface BottomPanelProps extends CommonProps {
children?: React.ReactNode;
/** Displayed when true. */
Expand Down Expand Up @@ -295,8 +298,6 @@ export function ResizableBottomPanel(props: ResizableBottomPanelProps) {
}
setTimeout(() => {
setFlickingDown(false);
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
}, 50 + (typeof autoCloseResult === "number" ? autoCloseResult : getCssVariableAsNumber("--mui-bottom-panel-animation-duration") * 1000));
return result;
});
Expand Down
4 changes: 1 addition & 3 deletions src/mobile-ui-react/CenterDiv.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps } from "@itwin/core-react";
import "./CenterDiv.scss";
import { CommonProps } from "./MobileUi";

/**
* Properties for {@link CenterDiv} component
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
interface CenterDivProps extends CommonProps, React.DOMAttributes<HTMLDivElement> {
/** Set to true to have the {@link CenterDiv} fill 100% of its parent. Default: no */
fill?: boolean;
Expand Down
8 changes: 2 additions & 6 deletions src/mobile-ui-react/CountNotification.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps, IconSpec } from "@itwin/core-react";
import { IconSpec } from "@itwin/core-react";
import { AlertAction } from "@itwin/mobile-sdk-core";
import { ActionSheetButton } from "./ActionSheetButton";
import { IconImage } from "./IconImage";
import { MobileUi } from "./MobileUi";
import { CommonProps, MobileUi } from "./MobileUi";

import "./CountNotification.scss";

Expand Down Expand Up @@ -40,8 +40,6 @@ export interface CountNotificationMoreProps {
* Properties for {@link CountNotification} component
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface CountNotificationProps extends CommonProps {
/** Count to display */
count: number;
Expand All @@ -67,8 +65,6 @@ export interface CountNotificationProps extends CommonProps {
* Properties for {@link CloseableCountNotification} component
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface CloseableCountNotificationProps extends CommonProps {
/** Count to display */
count: number;
Expand Down
6 changes: 1 addition & 5 deletions src/mobile-ui-react/HorizontalPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps } from "@itwin/core-react";
import { CommonProps } from "./MobileUi";

import "./HorizontalPicker.scss";

/**
* Properties for {@link HorizontalPicker} component
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface HorizontalPickerProps extends CommonProps {
/** The items in the picker. */
items: React.ReactNode[];
Expand All @@ -23,8 +21,6 @@ export interface HorizontalPickerProps extends CommonProps {
onItemSelected: (item: number) => void;
}

// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
interface HorizontalPickerItemProps extends CommonProps {
itemNode: React.ReactNode;
onClick: () => void;
Expand Down
35 changes: 29 additions & 6 deletions src/mobile-ui-react/MobileUi.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import * as React from "react";
import { BackendError, Localization } from "@itwin/core-common";
import {
ColorTheme,
SessionStateActionId,
SyncUiEventDispatcher,
SyncUiEventId,
SYSTEM_PREFERRED_COLOR_THEME,
UiFramework,
UiSyncEventArgs,
useActiveIModelConnection,
} from "@itwin/appui-react";
import { EmphasizeElements, IModelApp, IModelConnection, ScreenViewport, SelectionSet, Tool, Viewport } from "@itwin/core-frontend";
import { BeEvent, BeUiEvent, BriefcaseStatus, Id64Set, Listener } from "@itwin/core-bentley";
Expand All @@ -27,6 +27,30 @@ import {

import "./MobileUi.scss";

/** Props used by components that expect class name to be passed in.
*
* __Note__: Copied from @itwin/core-react, where it is being deprecated. It will **not ever** be
* deprecated from @itwin/mobile-ui-react.
* @public
*/
export interface ClassNameProps {
Copy link
Copy Markdown
Member

@aruniverse aruniverse Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets review with the appui team why this was deprecated, and what they expected consumers to do, before we re-export a deprecated type from another repo as public here

Copy link
Copy Markdown
Member Author

@tcobbs-bentley tcobbs-bentley Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to draft for now while we decide what to do and created a new PR to allow lint to pass.

/** Custom CSS class name */
className?: string;
}

/** Common props used by components.
*
* __Note__: Copied from @itwin/core-react, where it is being deprecated. It will **not ever** be
* deprecated from @itwin/mobile-ui-react.
* @public
*/
export interface CommonProps extends ClassNameProps {
/** Custom CSS style properties */
style?: React.CSSProperties;
/** Optional unique identifier for item. If defined it will be added to DOM Element attribute as data-item-id */
itemId?: string;
}

/** Type used for MobileUi.onClose BeEvent. */
export declare type CloseListener = () => void;

Expand Down Expand Up @@ -558,11 +582,10 @@ export function useIsolatedCount(): number {
* @param handler - The callback function.
*/
export function useIModel(handler: (iModel: IModelConnection | undefined) => void) {
useSyncUiEvent(React.useCallback(() => {
handler(UiFramework.getIModelConnection());
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
}, [handler]), SessionStateActionId.SetIModelConnection);
const iModelConnection = useActiveIModelConnection();
Copy link
Copy Markdown
Contributor

@toddsouthenbentley toddsouthenbentley Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest removing this function since it doesn't add anything that can't be accomplished with useActiveIModelConnection

Copy link
Copy Markdown
Member Author

@tcobbs-bentley tcobbs-bentley Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SYNCHRO Field uses it in one place, and they take advantage of the fact that they get a callback when it happens. They don't actually care about the value, simply that it changed:

  useIModel(() => {
    reset();
  });

Obviously they could update their code to React.useMemo like I do here. I guess we could ask them.

Copy link
Copy Markdown
Contributor

@toddsouthenbentley toddsouthenbentley Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest calling useEffect instead of useMemo since the latter is meant for memoizing calculated data (i.e. storing the return value of useMemo).

I think having Field update their code to call useActiveIModelConnection makes sense.

Copy link
Copy Markdown
Member Author

@tcobbs-bentley tcobbs-bentley Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If SYNCHRO agrees with us removing useIModel, this discussion is moot, but based on my understanding of how useEffect and useMemo work, useEffect will cause an extra render at startup and every time the value changes. Having said that, I am well aware that this is a very non-standard usage of useMemo, so maybe the extra render is not something to worry about.

React.useMemo(() => {
handler(iModelConnection);
}, [iModelConnection, handler]);
}

/**
Expand Down
5 changes: 1 addition & 4 deletions src/mobile-ui-react/ModalEntryFormDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps } from "@itwin/core-react";
import { UiFramework } from "@itwin/appui-react";
import { getCssVariableAsNumberOrDefault, MobileCore, Optional } from "@itwin/mobile-sdk-core";
import { CloseButton } from "./NavigationPanel";
import { MobileUi } from "./MobileUi";
import { CommonProps, MobileUi } from "./MobileUi";

import "./ModalEntryFormDialog.scss";

Expand Down Expand Up @@ -41,8 +40,6 @@ export interface ModalEntryFormValue {
* Properties for the {@link ModalDialog} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface ModalDialogProps extends CommonProps {
/** Content. */
children: React.ReactNode;
Expand Down
8 changes: 2 additions & 6 deletions src/mobile-ui-react/NavigationPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { ClassNameProps, CommonProps, IconSpec } from "@itwin/core-react";
import { IconSpec } from "@itwin/core-react";
import { ConditionalBooleanValue, ConditionalStringValue } from "@itwin/appui-abstract";
import { IconImage } from "./IconImage";
import { useSyncUiEvent } from "./MobileUi";
import { ClassNameProps, CommonProps, useSyncUiEvent } from "./MobileUi";
import "./NavigationPanel.scss";

import {
Expand All @@ -21,8 +21,6 @@ import {
* Properties for the {@link NavigationPanel} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface NavigationPanelProps extends ClassNameProps {
/** The left side components. */
left?: React.ReactNode;
Expand All @@ -46,8 +44,6 @@ export function NavigationPanel(props: NavigationPanelProps) {
* Properties for the {@link NavigationButton} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface NavigationButtonProps extends CommonProps {
/** The icon. */
iconSpec: IconSpec;
Expand Down
4 changes: 1 addition & 3 deletions src/mobile-ui-react/PanelHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps } from "@itwin/core-react";
import { withoutClassName } from "@itwin/mobile-sdk-core";
import { DraggableComponent, DraggableComponentCallbackProps } from "./ResizablePanel";
import { CommonProps } from "./MobileUi";

import "./PanelHeader.scss";

Expand Down Expand Up @@ -35,8 +35,6 @@ function PanelHeaderDraggableDiv(props: PanelHeaderDraggableDivProps) {
* Properties for the {@link PanelHeaderButton} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface PanelHeaderButtonProps extends CommonProps {
/** The button's text label. */
label: string;
Expand Down
11 changes: 1 addition & 10 deletions src/mobile-ui-react/ResizablePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@
import * as React from "react";
import classnames from "classnames";
import { Point2d, XAndY } from "@itwin/core-geometry";
import { CommonProps } from "@itwin/core-react";
import { getCssVariableAsNumber, ReloadedEvent } from "@itwin/mobile-sdk-core";
import { ReactUseState, useIsMountedRef, useWindowEvent } from "./MobileUi";
import { CommonProps, ReactUseState, useIsMountedRef, useWindowEvent } from "./MobileUi";
import "./ResizablePanel.scss";

/**
* Properties for {@link ResizablePanel} component
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface ResizablePanelProps extends CommonProps {
/** The children */
children?: React.ReactNode;
Expand Down Expand Up @@ -356,8 +353,6 @@ export function DraggableComponent(props: DraggableComponentProps) {
</TouchDragHandle>;
}

// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
interface VerticalScrollProps extends CommonProps {
children?: React.ReactNode;
}
Expand All @@ -368,8 +363,6 @@ export function VerticalScroll(props: VerticalScrollProps) {
return <div {...otherProps} className={classnames("mui-vertical-scroll", className)}>{children}</div>;
}

// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
interface TouchCaptorProps extends CommonProps {
isTouchStarted: boolean;
onTouchStart?: (e: TouchEvent) => void;
Expand Down Expand Up @@ -470,8 +463,6 @@ interface TouchDragHandleState {
isPointerDown: boolean;
}

// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
interface TouchDragHandleProps extends CommonProps {
/** Last pointer position of draggable tab. */
lastPosition?: XAndY;
Expand Down
5 changes: 1 addition & 4 deletions src/mobile-ui-react/ScrollableWithFades.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import * as React from "react";
import classnames from "classnames";
import { ColorDef } from "@itwin/core-common";
import { ClassNameProps } from "@itwin/core-react";
import { useScroll, useWindowEvent } from "./MobileUi";
import { ClassNameProps, useScroll, useWindowEvent } from "./MobileUi";
import { getCssVariable } from "@itwin/mobile-sdk-core";

import "./ScrollableWithFades.scss";
Expand All @@ -17,8 +16,6 @@ import "./ScrollableWithFades.scss";
* Properties for the {@link HorizontalScrollableWithFades} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface ScrollableWithFadesProps extends ClassNameProps {
backgroundColor?: ColorDef;
/** The views to go into the scrollable element */
Expand Down
4 changes: 1 addition & 3 deletions src/mobile-ui-react/Suggestion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
import * as React from "react";
import classnames from "classnames";
import { BeUiEvent } from "@itwin/core-bentley";
import { useOnOutsideClick } from "@itwin/core-react";
import { ToolAssistanceInstructions } from "@itwin/core-frontend";
import { IconImage } from "./IconImage";
import { useBeUiEvent } from "./MobileUi";
import { useOnOutsideClick } from "./useOnOutsideClick";
import { getCssVariableAsNumber } from "@itwin/mobile-sdk-core";

import "./Suggestion.scss";
Expand Down Expand Up @@ -39,8 +39,6 @@ export function SuggestionContainer(props: React.HTMLAttributes<HTMLDivElement>)
* @public
*/
export function Suggestion(props: SuggestionProps) {
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
const ref = useOnOutsideClick<HTMLDivElement>(() => props.onOutsideClick?.());
// To ensure the icon is properly centered when the label not displayed, calculate the icon's margin so that its height and width match the pill height.
const pillHeight = React.useRef(getCssVariableAsNumber("--mui-pill-height"));
Expand Down
7 changes: 1 addition & 6 deletions src/mobile-ui-react/TileGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import * as React from "react";
import classnames from "classnames";
import { CommonProps } from "@itwin/core-react";
import { useMediaQuery, useScrolling } from "./MobileUi";
import { CommonProps, useMediaQuery, useScrolling } from "./MobileUi";
import "./TileGrid.scss";

/**
Expand All @@ -25,8 +24,6 @@ export interface GridTileSize {
* Properties for {@link TileGrid} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface TileGridProps extends CommonProps {
/** [[GridTile]] children of this node */
children?: Array<React.ReactElement<GridTileProps>>;
Expand Down Expand Up @@ -55,8 +52,6 @@ export interface GridTileInjectedProps {
* Properties for {@link GridTile} component.
* @public
*/
// @todo AppUI deprecation
// eslint-disable-next-line deprecation/deprecation
export interface GridTileProps extends CommonProps {
/** onClick handler for this GridTile. */
onClick?: (e: React.MouseEvent) => void;
Expand Down
Loading