From 703084bbb1994516da06066a7f292ffe396955d4 Mon Sep 17 00:00:00 2001 From: Marcus Notheis Date: Tue, 9 Jul 2024 15:21:58 +0200 Subject: [PATCH] refactor(ObjectPage): api alignment (#6047) BREAKING CHANGE: the props `showHideHeaderButton` and `showTitleInHeaderContent` have been removed. BREAKING CHANGE: the prop `alwaysShowContentHeader` has been renamed to `headerPinned` BREAKING CHANGE: the prop `headerContentPinnable` has been renamed to `hidePinButton` and its logic has been inverted. The pin button is now shown by default. BREAKING CHANGE: the prop `showSubHeaderRight` has been removed as it's not defined by design anymore. --------- Co-authored-by: Lukas Harbarth --- docs/MigrationGuide.mdx | 45 ++++-- .../codemod/transforms/v2/codemodConfig.json | 7 +- .../scripts/codemod/transforms/v2/main.cts | 27 ++++ .../components/ObjectPage/ObjectPage.cy.tsx | 31 ++-- .../ObjectPage/ObjectPage.module.css | 5 +- .../ObjectPage/ObjectPage.stories.tsx | 7 +- .../main/src/components/ObjectPage/index.tsx | 146 ++++-------------- .../components/ObjectPageAnchorBar/index.tsx | 53 +++---- .../src/components/ObjectPageTitle/index.tsx | 12 +- 9 files changed, 131 insertions(+), 202 deletions(-) diff --git a/docs/MigrationGuide.mdx b/docs/MigrationGuide.mdx index 7002bd64a21..8abda876c14 100644 --- a/docs/MigrationGuide.mdx +++ b/docs/MigrationGuide.mdx @@ -395,21 +395,6 @@ import { Loader } from '@ui5/webcomponents-react'; import { Loader } from '@ui5/webcomponents-react-compat'; ``` -### ObjectPage - -The newly introduced `DynamicPage` web component comes with its own `DynamicPageHeader` and `DynamicPageTitle` components, which are unfortunately incompatible with our `ObjectPage` implementation. -Please use the following components instead: - -- `headerContent` now only accepts the `ObjectPageHeader` component. -- `headerTitle` now only accepts the `ObjectPageTitle` component. - -**Renamed Props:** - -- `a11yConfig` has been renamed to `accessibilityAttributes` -- `a11yConfig.dynamicPageAnchorBar` has been renamed to `accessibilityAttributes.objectPageAnchorBar` - -Also, the namings of internal `data-component-name` attributes have been adjusted accordingly. E.g. `data-component-name="DynamicPageTitleSubHeader"` has been renamed to `data-component-name="ObjectPageTitleSubHeader"` - ### Text The `Text` component has been replaced with the `ui5-text` Web Component. @@ -623,6 +608,36 @@ All Modal helper hooks have been removed. They can be replaced with the regular The regular methods are now general purpose, so they can be used both inside the React content (components) as well as outside of the React context (redux, redux-saga, etc.). +### ObjectPage + +The newly introduced `DynamicPage` web component comes with its own `DynamicPageHeader` and `DynamicPageTitle` components, which are unfortunately incompatible with our `ObjectPage` implementation. +Please use the following components instead: + +- `headerContent` now only accepts the `ObjectPageHeader` component. +- `headerTitle` now only accepts the `ObjectPageTitle` component. + +**Renamed Props:** + +- `a11yConfig` has been renamed to `accessibilityAttributes` +- `a11yConfig.dynamicPageAnchorBar` has been renamed to `accessibilityAttributes.objectPageAnchorBar` +- `alwaysShowContentHeader` has been renamed to `headerPinned` +- `headerContentPinnable` has been renamed to `hidePinButton` and the logic has been inverted. The pin button is now shown by default. + +**Removed Props:** + +- `showHideHeaderButton`: Hiding the expand/collapse button is not supported by design anymore. +- `showTitleInHeaderContent`: Showing the `headerTitle` as part of the `headerContent` is [not supported by design anymore](https://experience.sap.com/fiori-design-web/object-page/#dynamic-page-header-mandatory). + +Also, the namings of internal `data-component-name` attributes have been adjusted accordingly. E.g. `data-component-name="DynamicPageTitleSubHeader"` has been renamed to `data-component-name="ObjectPageTitleSubHeader"` + +### ObjectPageTitle + +_The `ObjectPageTitle` component is the renamed implementation of the old (React only) `DynamicPageTitle` component._ + +**Removed Props:** + +- `showSubHeaderRight`: Displaying the subheader in the same line as the header is not supported by design anymore. + ### ObjectPageSection The prop `titleText` is now required and the default value `true` has been removed for the `titleTextUppercase` prop to comply with the updated Fiori design guidelines. diff --git a/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json b/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json index e5aff80fb45..19e09d0c14c 100644 --- a/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json +++ b/packages/cli/src/scripts/codemod/transforms/v2/codemodConfig.json @@ -293,8 +293,11 @@ }, "ObjectPage": { "changedProps": { - "a11yConfig": "accessibilityAttributes" - } + "a11yConfig": "accessibilityAttributes", + "alwaysShowContentHeader": "headerPinned", + "headerContentPinnable": "hidePinButton" + }, + "removedProps": ["showHideHeaderButton", "showTitleInHeaderContent"] }, "ObjectStatus": { "changedProps": { diff --git a/packages/cli/src/scripts/codemod/transforms/v2/main.cts b/packages/cli/src/scripts/codemod/transforms/v2/main.cts index 0ff9731e39f..1ce487ae1b6 100644 --- a/packages/cli/src/scripts/codemod/transforms/v2/main.cts +++ b/packages/cli/src/scripts/codemod/transforms/v2/main.cts @@ -323,6 +323,33 @@ export default function transform(file: FileInfo, api: API, options?: Options): }); } + if (componentName === 'ObjectPage') { + jsxElements.forEach((el) => { + const headerContentPinnable = j(el).find(j.JSXAttribute, { name: { name: 'headerContentPinnable' } }); + if (headerContentPinnable.size() === 0) { + j(el) + .find(j.JSXOpeningElement) + .get() + .value.attributes.push(j.jsxAttribute(j.jsxIdentifier('hidePinButton'), null)); + isDirty = true; + } else { + const attr = headerContentPinnable.get(); + if ( + attr.value.value && + ((attr.value.value.type === 'JSXAttribute' && attr.value.value === false) || + (attr.value.value.type === 'JSXExpressionContainer' && attr.value.value.expression.value === false)) + ) { + j(el) + .find(j.JSXOpeningElement) + .get() + .value.attributes.push(j.jsxAttribute(j.jsxIdentifier('hidePinButton'), null)); + } + headerContentPinnable.remove(); + isDirty = true; + } + }); + } + if (componentName === 'Page') { jsxElements.forEach((el) => { const floatingFooter = j(el).find(j.JSXAttribute, { name: { name: 'floatingFooter' } }); diff --git a/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx b/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx index 08b94b49a10..d428e21cf71 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPage.cy.tsx @@ -43,8 +43,7 @@ describe('ObjectPage', () => { headerTitle={} headerContent={ObjectPageHeader} onToggleHeaderContent={toggle} - headerContentPinnable={false} - showHideHeaderButton + hidePinButton > Content @@ -81,8 +80,6 @@ describe('ObjectPage', () => { style={{ height: '100vh' }} headerTitle={} headerContent={ObjectPageHeader} - headerContentPinnable - showHideHeaderButton onPinnedStateChange={pin} data-testid="op" > @@ -111,7 +108,7 @@ describe('ObjectPage', () => { cy.findByText('ObjectPageHeader').should('not.be.visible'); }); - it('programmatically pin header (`alwaysShowContentHeader`)', () => { + it('programmatically pin header (`headerPinned`)', () => { document.body.style.margin = '0px'; const TestComp = ({ onPinnedStateChange }: ObjectPagePropTypes) => { const [pinned, setPinned] = useState(false); @@ -133,9 +130,7 @@ describe('ObjectPage', () => { style={{ height: '95vh' }} headerTitle={} headerContent={ObjectPageHeader} - headerContentPinnable - showHideHeaderButton - alwaysShowContentHeader={pinned} + headerPinned={pinned} onPinnedStateChange={handlePinChange} data-testid="op" > @@ -212,8 +207,6 @@ describe('ObjectPage', () => {
ObjectPageHeader
} - headerContentPinnable - showHideHeaderButton data-testid="op" > @@ -394,7 +387,6 @@ describe('ObjectPage', () => { headerTitle={DPTitle} headerContent={DPContent} data-testid="op" - showHideHeaderButton ref={ref} footer={withFooter && Footer} mode={mode} @@ -420,19 +412,19 @@ describe('ObjectPage', () => { }; cy.mount(); cy.findByText('Update Heights').click(); - cy.findByText('{"offset":1080,"scroll":2240}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2260}').should('exist'); cy.findByTestId('op').scrollTo('bottom'); cy.findByText('Update Heights').click({ force: true }); - cy.findByText('{"offset":1080,"scroll":2240}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2260}').should('exist'); cy.mount(); cy.findByText('Update Heights').click(); - cy.findByText('{"offset":1080,"scroll":2290}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2310}').should('exist'); cy.findByTestId('op').scrollTo('bottom'); cy.findByText('Update Heights').click({ force: true }); - cy.findByText('{"offset":1080,"scroll":2290}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2310}').should('exist'); cy.mount(); cy.findByText('Update Heights').click(); @@ -492,7 +484,6 @@ describe('ObjectPage', () => { headerTitle={DPTitle} headerContent={DPContent} data-testid="op" - showHideHeaderButton ref={ref} footer={withFooter && Footer} mode={mode} @@ -518,19 +509,19 @@ describe('ObjectPage', () => { }; cy.mount(); cy.findByText('Update Heights').click(); - cy.findByText('{"offset":1080,"scroll":2240}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2260}').should('exist'); cy.findByTestId('op').scrollTo('bottom'); cy.findByText('Update Heights').click({ force: true }); - cy.findByText('{"offset":1080,"scroll":2240}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2260}').should('exist'); cy.mount(); cy.findByText('Update Heights').click(); - cy.findByText('{"offset":1080,"scroll":2300}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2320}').should('exist'); cy.findByTestId('op').scrollTo('bottom'); cy.findByText('Update Heights').click({ force: true }); - cy.findByText('{"offset":1080,"scroll":2300}').should('exist'); + cy.findByText('{"offset":1080,"scroll":2320}').should('exist'); cy.mount(); cy.findByText('Update Heights').click(); diff --git a/packages/main/src/components/ObjectPage/ObjectPage.module.css b/packages/main/src/components/ObjectPage/ObjectPage.module.css index 37bffec20cd..65bb1e7980f 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.module.css +++ b/packages/main/src/components/ObjectPage/ObjectPage.module.css @@ -41,6 +41,7 @@ inset-block-start: 0; z-index: 2; cursor: pointer; + display: grid; [data-component-name='ObjectPageTitle'] { grid-column: 2; @@ -171,10 +172,6 @@ padding: 0; } -.titleInHeader { - padding: 0; -} - .snappedContent { grid-column: 1 / span 2; } diff --git a/packages/main/src/components/ObjectPage/ObjectPage.stories.tsx b/packages/main/src/components/ObjectPage/ObjectPage.stories.tsx index d5e2c0be211..7654546b3e6 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.stories.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPage.stories.tsx @@ -32,7 +32,7 @@ import { Text, Title, ToggleButton -} from '../..'; +} from '../../index.js'; import { ObjectPage } from './index.js'; const meta = { @@ -48,9 +48,7 @@ const meta = { }, args: { mode: ObjectPageMode.Default, - showHideHeaderButton: true, selectedSectionId: 'goals', - headerContentPinnable: true, imageShapeCircle: true, image: SampleImage, style: { height: '700px' }, @@ -67,7 +65,6 @@ const meta = { ), headerTitle: ( { * * __Note:__ Although this prop accepts all HTML Elements, it is strongly recommended that you only use `ObjectPageTitle` in order to preserve the intended design. * - * __Note:__ If not defined otherwise the prop `showSubHeaderRight` of the `ObjectPageTitle` is set to `true` by default. - * * __Note:__ When the `ObjectPageTitle` is rendered inside a custom component, it's essential to pass through all props, as otherwise the component won't function as intended! */ headerTitle?: ReactElement; @@ -115,15 +111,9 @@ export interface ObjectPagePropTypes extends Omit { */ selectedSubSectionId?: string; /** - * Defines whether the `headerContent` is hidden by scrolling down. - */ - alwaysShowContentHeader?: boolean; - /** - * Defines whether the title is displayed in the content section of the header or above the image. - * - * @deprecated: This feature will be removed with our next major release. + * Defines whether the `headerContent` is pinned. */ - showTitleInHeaderContent?: boolean; + headerPinned?: boolean; /** * Defines whether the image should be displayed in a circle or in a square.
* __Note:__ If the `image` is not a `string`, this prop has no effect. @@ -139,13 +129,9 @@ export interface ObjectPagePropTypes extends Omit { */ mode?: ObjectPageMode | keyof typeof ObjectPageMode; /** - * Defines whether the pin button of the header is displayed. + * Defines if the pin button for the `headerContent` is hidden. */ - showHideHeaderButton?: boolean; - /** - * Defines whether the `headerContent` is pinnable. - */ - headerContentPinnable?: boolean; + hidePinButton?: boolean; /** * Defines internally used accessibility properties/attributes. */ @@ -201,13 +187,11 @@ const ObjectPage = forwardRef((props, ref) className, style, slot, - showHideHeaderButton, children, selectedSectionId, - alwaysShowContentHeader, - showTitleInHeaderContent, + headerPinned: headerPinnedProp, headerContent, - headerContentPinnable, + hidePinButton, accessibilityAttributes, placeholder, onSelectedSectionChange, @@ -226,7 +210,7 @@ const ObjectPage = forwardRef((props, ref) selectedSectionId ?? firstSectionId ); const [selectedSubSectionId, setSelectedSubSectionId] = useState(props.selectedSubSectionId); - const [headerPinned, setHeaderPinned] = useState(alwaysShowContentHeader); + const [headerPinned, setHeaderPinned] = useState(headerPinnedProp); const isProgrammaticallyScrolled = useRef(false); const prevSelectedSectionId = useRef(undefined); @@ -244,22 +228,10 @@ const ObjectPage = forwardRef((props, ref) const [headerCollapsedInternal, setHeaderCollapsedInternal] = useState(undefined); const [scrolledHeaderExpanded, setScrolledHeaderExpanded] = useState(false); const scrollTimeout = useRef(0); - const titleInHeader = headerTitle && showTitleInHeaderContent; const [sectionSpacer, setSectionSpacer] = useState(0); const [currentTabModeSection, setCurrentTabModeSection] = useState(null); const sections = mode === ObjectPageMode.IconTabBar ? currentTabModeSection : children; - const deprecationNoticeDisplayed = useRef(false); - useEffect(() => { - if (showTitleInHeaderContent && !deprecationNoticeDisplayed.current) { - deprecationNotice( - 'showTitleInHeaderContent', - 'showTitleInHeaderContent is deprecated and will be removed with the next major release.' - ); - deprecationNoticeDisplayed.current = true; - } - }, [showTitleInHeaderContent]); - useEffect(() => { const currentSection = mode === ObjectPageMode.IconTabBar ? getSectionById(children, internalSelectedSectionId) : null; @@ -433,13 +405,13 @@ const ObjectPage = forwardRef((props, ref) }, [selectedSubSectionId, isProgrammaticallyScrolled.current, sectionSpacer]); useEffect(() => { - if (alwaysShowContentHeader !== undefined) { - setHeaderPinned(alwaysShowContentHeader); + if (headerPinnedProp !== undefined) { + setHeaderPinned(headerPinnedProp); } - if (alwaysShowContentHeader) { + if (headerPinnedProp) { onToggleHeaderContentVisibility({ detail: { visible: true } }); } - }, [alwaysShowContentHeader]); + }, [headerPinnedProp]); const prevHeaderPinned = useRef(headerPinned); useEffect(() => { @@ -626,49 +598,16 @@ const ObjectPage = forwardRef((props, ref) } }, [isAfterScroll]); - const titleHeaderNotClickable = - (alwaysShowContentHeader && !headerContentPinnable) || - !headerContent || - (!showHideHeaderButton && !headerContentPinnable); - const onTitleClick = useCallback( (e) => { e.stopPropagation(); - if (!titleHeaderNotClickable) { - onToggleHeaderContentVisibility(enrichEventWithDetails(e, { visible: headerCollapsed })); - } + onToggleHeaderContentVisibility(enrichEventWithDetails(e, { visible: headerCollapsed })); }, - [onToggleHeaderContentVisibility, headerCollapsed, titleHeaderNotClickable] + [onToggleHeaderContentVisibility, headerCollapsed] ); const snappedHeaderInObjPage = headerTitle && headerTitle.props.snappedContent && headerCollapsed === true && !!image; - const hasHeaderContent = !!headerContent; - const renderTitleSection = useCallback( - (inHeader = false) => { - const titleInHeaderClass = inHeader ? classNames.titleInHeader : undefined; - - if (headerTitle?.props && headerTitle.props?.showSubHeaderRight === undefined) { - return cloneElement(headerTitle as ReactElement, { - showSubHeaderRight: true, - className: clsx(titleInHeaderClass, headerTitle?.props?.className), - onToggleHeaderContentVisibility: onTitleClick, - 'data-not-clickable': titleHeaderNotClickable, - 'data-header-content-visible': headerContent && headerCollapsed !== true, - 'data-is-snapped-rendered-outside': snappedHeaderInObjPage - }); - } - return cloneElement(headerTitle as ReactElement, { - className: clsx(titleInHeaderClass, headerTitle?.props?.className), - onToggleHeaderContentVisibility: onTitleClick, - 'data-not-clickable': titleHeaderNotClickable, - 'data-header-content-visible': headerContent && headerCollapsed !== true, - 'data-is-snapped-rendered-outside': snappedHeaderInObjPage - }); - }, - [headerTitle, titleHeaderNotClickable, onTitleClick, headerCollapsed, snappedHeaderInObjPage, hasHeaderContent] - ); - const isInitial = useRef(true); useEffect(() => { if (!isInitial.current) { @@ -693,40 +632,14 @@ const ObjectPage = forwardRef((props, ref) children: (
{avatar} - {(headerContent.props.children || titleInHeader) && ( -
- {titleInHeader && renderTitleSection(true)} - {headerContent.props.children} -
+ {headerContent.props.children && ( +
{headerContent.props.children}
)}
) }); - } else if (titleInHeader) { - return ( - -
- {avatar} -
{titleInHeader && renderTitleSection(true)}
-
- - ); } - }, [ - headerContent, - topHeaderHeight, - headerPinned, - scrolledHeaderExpanded, - titleInHeader, - avatar, - headerContentRef, - renderTitleSection - ]); + }, [headerContent, topHeaderHeight, headerPinned, scrolledHeaderExpanded, avatar, headerContentRef]); const onTabItemSelect = (event) => { if (typeof onBeforeNavigate === 'function') { @@ -810,7 +723,7 @@ const ObjectPage = forwardRef((props, ref) const objectPageStyles: CSSProperties = { ...style }; - if (headerCollapsed === true && (headerContent || titleInHeader)) { + if (headerCollapsed === true && headerContent) { objectPageStyles[ObjectPageCssVariables.titleFontSize] = ThemingParameters.sapObjectHeader_Title_SnappedFontSize; } @@ -830,21 +743,27 @@ const ObjectPage = forwardRef((props, ref) data-component-name="ObjectPageTopHeader" ref={topHeaderRef} role={accessibilityAttributes?.objectPageTopHeader?.role} - data-not-clickable={titleHeaderNotClickable} + data-not-clickable={false} aria-roledescription={accessibilityAttributes?.objectPageTopHeader?.ariaRoledescription ?? 'Object Page header'} className={classNames.header} onClick={onTitleClick} style={{ gridAutoColumns: `min-content ${ headerTitle && image && headerCollapsed === true ? `calc(100% - 3rem - 1rem)` : '100%' - }`, - display: !showTitleInHeaderContent || headerCollapsed === true ? 'grid' : 'none' + }` }} > {headerTitle && image && headerCollapsed === true && ( )} - {headerTitle && renderTitleSection()} + {headerTitle && + cloneElement(headerTitle as ReactElement, { + className: clsx(headerTitle?.props?.className), + onToggleHeaderContentVisibility: onTitleClick, + 'data-not-clickable': false, + 'data-header-content-visible': headerContent && headerCollapsed !== true, + 'data-is-snapped-rendered-outside': snappedHeaderInObjPage + })} {snappedHeaderInObjPage && (
{headerTitle.props.snappedContent} @@ -866,8 +785,7 @@ const ObjectPage = forwardRef((props, ref) > ((props, ref) => { const { - showHideHeaderButton, headerContentVisible, - headerContentPinnable, + hidePinButton, headerPinned, style, accessibilityAttributes, @@ -85,8 +80,8 @@ const ObjectPageAnchorBar = forwardRef(null); - const shouldRenderHeaderPinnableButton = headerContentPinnable && headerContentVisible; - const showBothActions = shouldRenderHeaderPinnableButton && showHideHeaderButton; + const shouldRenderHeaderPinnableButton = !hidePinButton && headerContentVisible; + const showBothActions = shouldRenderHeaderPinnableButton; const onPinHeader = useCallback( (e) => { @@ -124,28 +119,26 @@ const ObjectPageAnchorBar = forwardRef - {showHideHeaderButton && ( -
)} - {subHeader && showSubHeaderRight && ( -
- {subHeader} -
- )} {children && (
{children} @@ -312,7 +302,7 @@ const ObjectPageTitle = forwardRef((pr )} - {subHeader && !showSubHeaderRight && ( + {subHeader && (