Skip to content
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.

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

refactor(ObjectPage): api alignment #6047

Merged
merged 7 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 30 additions & 15 deletions docs/MigrationGuide.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,11 @@
},
"ObjectPage": {
"changedProps": {
"a11yConfig": "accessibilityAttributes"
}
"a11yConfig": "accessibilityAttributes",
"alwaysShowContentHeader": "headerPinned",
"headerContentPinnable": "hidePinButton"
},
"removedProps": ["showHideHeaderButton", "showTitleInHeaderContent"]
},
"ObjectStatus": {
"changedProps": {
Expand Down
27 changes: 27 additions & 0 deletions packages/cli/src/scripts/codemod/transforms/v2/main.cts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } });
Expand Down
31 changes: 11 additions & 20 deletions packages/main/src/components/ObjectPage/ObjectPage.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
>
Expand Down Expand Up @@ -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);
Expand All @@ -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"
>
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -394,7 +387,6 @@ describe('ObjectPage', () => {
headerTitle={DPTitle}
headerContent={DPContent}
data-testid="op"
showHideHeaderButton
ref={ref}
footer={withFooter && Footer}
mode={mode}
Expand All @@ -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();
Expand Down Expand Up @@ -492,7 +484,6 @@ describe('ObjectPage', () => {
headerTitle={DPTitle}
headerContent={DPContent}
data-testid="op"
showHideHeaderButton
ref={ref}
footer={withFooter && Footer}
mode={mode}
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
inset-block-start: 0;
z-index: 2;
cursor: pointer;
display: grid;

[data-component-name='ObjectPageTitle'] {
grid-column: 2;
Expand Down Expand Up @@ -171,10 +172,6 @@
padding: 0;
}

.titleInHeader {
padding: 0;
}

.snappedContent {
grid-column: 1 / span 2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
Text,
Title,
ToggleButton
} from '../..';
} from '../../index.js';
import { ObjectPage } from './index.js';

const meta = {
Expand All @@ -48,9 +48,7 @@ const meta = {
},
args: {
mode: ObjectPageMode.Default,
showHideHeaderButton: true,
selectedSectionId: 'goals',
headerContentPinnable: true,
imageShapeCircle: true,
image: SampleImage,
style: { height: '700px' },
Expand All @@ -67,7 +65,6 @@ const meta = {
),
headerTitle: (
<ObjectPageTitle
showSubHeaderRight
header="Denise Smith"
subHeader="Senior UI Developer"
actions={
Expand Down Expand Up @@ -337,7 +334,6 @@ export const WithCustomOverflowButton: Story = {
<>
<ObjectPage
style={{ width: '1000px' }}
showHideHeaderButton={false}
headerTitle={
<ObjectPageTitle
{...titleProps}
Expand All @@ -351,7 +347,6 @@ export const WithCustomOverflowButton: Story = {
/>
<ObjectPage
style={{ width: '1400px' }}
showHideHeaderButton={false}
headerTitle={
<ObjectPageTitle
{...titleProps}
Expand Down
Loading
Loading