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 <lukas.harbarth@sap.com>
SAP · Jul 9, 2024 · 703084b · 703084b
1 parent e0818c4
commit 703084b
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 202 deletions.
diff --git 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
@@ -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
@@ -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
@@ -43,8 +43,7 @@ describe('ObjectPage', () => {
         headerTitle={<ObjectPageTitle header="Heading" subHeader="SubHeading" />}
         headerContent={<ObjectPageHeader>ObjectPageHeader</ObjectPageHeader>}
         onToggleHeaderContent={toggle}
-        headerContentPinnable={false}
-        showHideHeaderButton
+        hidePinButton
       >
         <ObjectPageSection id="section" titleText="Section">
           Content
@@ -81,8 +80,6 @@ describe('ObjectPage', () => {
         style={{ height: '100vh' }}
         headerTitle={<ObjectPageTitle header="Heading" subHeader="SubHeading" />}
         headerContent={<ObjectPageHeader>ObjectPageHeader</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={<ObjectPageTitle header="Heading" subHeader="SubHeading" />}
             headerContent={<ObjectPageHeader>ObjectPageHeader</ObjectPageHeader>}
-            headerContentPinnable
-            showHideHeaderButton
-            alwaysShowContentHeader={pinned}
+            headerPinned={pinned}
             onPinnedStateChange={handlePinChange}
             data-testid="op"
           >
@@ -212,8 +207,6 @@ describe('ObjectPage', () => {
             <div style={{ height: '400px', width: '100%', background: 'lightyellow' }}>ObjectPageHeader</div>
           </ObjectPageHeader>
         }
-        headerContentPinnable
-        showHideHeaderButton
         data-testid="op"
       >
         <ObjectPageSection id="section" titleText="Section">
@@ -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(<TestComp height="2000px" mode={ObjectPageMode.Default} />);
     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(<TestComp height="2000px" withFooter mode={ObjectPageMode.Default} />);
     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(<TestComp height="400px" mode={ObjectPageMode.Default} />);
     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(<TestComp height="2000px" mode={ObjectPageMode.IconTabBar} />);
     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(<TestComp height="2000px" withFooter mode={ObjectPageMode.IconTabBar} />);
     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(<TestComp height="400px" mode={ObjectPageMode.IconTabBar} />);
     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
@@ -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
@@ -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: (
       <ObjectPageTitle
-        showSubHeaderRight
         header="Denise Smith"
         subHeader="Senior UI Developer"
         actions={
@@ -337,7 +334,6 @@ export const WithCustomOverflowButton: Story = {
       <>
         <ObjectPage
           style={{ width: '1000px' }}
-          showHideHeaderButton={false}
           headerTitle={
             <ObjectPageTitle
               {...titleProps}
@@ -351,7 +347,6 @@ export const WithCustomOverflowButton: Story = {
         />
         <ObjectPage
           style={{ width: '1400px' }}
-          showHideHeaderButton={false}
           headerTitle={
             <ObjectPageTitle
               {...titleProps}