Set up DataView layout #5

fhlavac · 2024-05-06T12:35:32Z

RHCLOUD-32249

set up a data view stack layout
created a base for the DataViewToolbar
updated docs & tests

patternfly-build · 2024-05-06T12:37:28Z

Preview: https://react-data-view-pr-data-view-5.surge.sh

A11y report: https://react-data-view-pr-data-view-5-a11y.surge.sh

fhlavac · 2024-05-06T12:40:54Z

@nicolethoen @dlabaj with PF6, I've encountered two visual issues

extra space after the Toolbar component

the div with pf-v6-c-toolbar__content pf-m-hidden was not visible in PF5 but is shown in PF6
aligning items in PF Toolbar does not seem to work
the pagination ToolbarItem has align={{ default: 'alignRight' }}, but is still aligned left

Do you know if these issues are known? Shall I open issues for these? Thank you

fhlavac · 2024-05-06T13:02:07Z

packages/module/patternfly-docs/content/extensions/data-view/examples/DataView/DataView.md


-A blank example of the data view layout.
+You can make use of the predefined layout components to display a default header and footer. See [data view toolbar](/data-view/data-view-toolbar) for more information 


@nicolethoen I also see the a11y tests failing there with link-in-text-block. The link is added the same way as in the older docs framework version. Did anything change with v6?

You are not the only person who's run into this error during the migration to the v6 alphas. I will have to investigate how others have resolved this issue.

I think the recommended solution (since the rule being flagged by the a11y tests is a 'new experimental rule') is to ignore it.
You can see that being done in another extension here: patternfly/react-catalog-view@bd81ee6

Cool, thank you 🙂

nicolethoen · 2024-05-06T17:14:01Z

the div with pf-v6-c-toolbar__content pf-m-hidden was not visible in PF5 but is shown in PF6

this does appear to be a bug on v6 right now! if you wouldn't mind opening a v6 alpha bug that would be super helpful!

aligning items in PF Toolbar does not seem to work
the pagination ToolbarItem has align={{ default: 'alignRight' }}, but is still aligned left

It looks to me that rather than <ToolbarItem align={{ default: 'alignRight' }}> for the pagination ToolbarItem, you'll want <ToolbarItem variant='pagination'>. I agree the align={{ default: 'alignRight' }} does not seem to have any affect. That might also be a worthwhile bug to file as a v6 alpha bug. If it was intended, then we should update the documentation :)

Hyperkid123 · 2024-05-07T06:45:48Z

packages/module/src/DataViewToolbar/DataViewToolbar.tsx

+export const DataViewToolbar: React.FunctionComponent<DataViewToolbarProps> = ({ className, ouiaId = 'DataViewToolbar', page, perPage, isBottom }: DataViewToolbarProps) => (
+  <Toolbar ouiaId={ouiaId} className={className}>
+    <ToolbarContent>
+      {page && perPage && (


I think we might want to pass a prop pagination that will have the object shape. That way we can have a bit more strict typing while keeping the feature optional.

Hyperkid123 · 2024-05-07T06:46:03Z

packages/module/src/Hooks/useDataViewPagination.ts

@@ -0,0 +1,4 @@
+export interface DataViewPaginationProps {
+    page?: number;


This should not be optional.

@Hyperkid123 do you think we want to make perPage mandatory as well? I wonder if our implementation should contain a default value for that or if we want to let that up to the user of our component

I don't think we will be able to "guess" defaults that would make sense.

Hyperkid123 · 2024-05-07T06:47:04Z

packages/module/src/DataViewToolbar/DataViewToolbar.tsx

Just a question. Do you plan all of the toolbar features to be object props? Or do we expect some nodes or children to be passed?
(I understand this is very early on)

@Hyperkid123 I planned to use the node approach for actions - the example is mentioned in the ADR. Do you think it is worth applying also somewhere else? Thank you

I think we want to use one system everywhere. but I think it will get clearer once we move forwards with more.

Hyperkid123 · 2024-05-07T06:48:21Z

packages/module/src/DataView/DataView.tsx

-);
+  <Stack data-ouia-component-id={`${ouiaId}-stack}`}>
+    {React.Children.map(children, (child, index) => (
+      React.isValidElement(child) ? (


I think we want to throw an error if the element is valid. There is no point passing things that are not elements and acting as if it was OK.

Hyperkid123 · 2024-05-07T06:49:30Z

packages/module/src/DataView/DataView.tsx

-  text?: string;
+export interface DataViewProps extends DataViewPaginationProps {
+  /** Content rendered inside the data view */
+  children?: React.ReactNode;


This should be mandatory. The view serves no purpose if there are no children.

Also, you should use React.PropsWithChildren generic so you don't have to add types for children (when they are optional).

fhlavac · 2024-05-07T08:16:32Z

It looks to me that rather than <ToolbarItem align={{ default: 'alignRight' }}> for the pagination ToolbarItem, you'll want . I agree the align={{ default: 'alignRight' }} does not seem to have any affect. That might also be a worthwhile bug to file as a v6 alpha bug. If it was intended, then we should update the documentation :)

@nicolethoen thank you! I've missed the variant prop. It fixes the alignment, I've also opened those issues.

fhlavac · 2024-05-13T20:26:21Z

@Hyperkid123 all comments should be addressed, can you please take a look?

Hyperkid123

I think this is good as a base 👍

github-actions · 2024-05-14T08:59:13Z

🎉 This PR is included in version 1.0.0-prerelease.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2024-07-31T20:58:13Z

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

fhlavac added 3 commits May 6, 2024 14:30

Update PF version

bb12269

feat(layout): Set up the DataView layout

dea98b2

Update tests

046ed8f

fhlavac commented May 6, 2024

View reviewed changes

Do not pass props to children

72be342

fhlavac requested review from Hyperkid123 and nicolethoen May 6, 2024 15:37

Hyperkid123 reviewed May 7, 2024

View reviewed changes

feat(layout): Clean the code

dfac428

fhlavac requested a review from Hyperkid123 May 7, 2024 09:45

fhlavac added 2 commits May 8, 2024 09:26

Ignore link-in-text-block a11y rule

6664871

Make the Toolbar consume pagination node

789eafd

Hyperkid123 approved these changes May 14, 2024

View reviewed changes

fhlavac merged commit 20d290c into patternfly:main May 14, 2024
7 checks passed

fhlavac deleted the layout branch May 14, 2024 08:54

github-actions bot added the released on @prerelease label May 14, 2024

github-actions bot added the released on @prerelease-v5 label Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up DataView layout #5

Set up DataView layout #5

fhlavac commented May 6, 2024 •

edited

Loading

patternfly-build commented May 6, 2024 •

edited

Loading

fhlavac commented May 6, 2024 •

edited

Loading

fhlavac May 6, 2024 •

edited

Loading

nicolethoen May 7, 2024

nicolethoen May 7, 2024

fhlavac May 8, 2024

nicolethoen commented May 6, 2024

Hyperkid123 May 7, 2024

Hyperkid123 May 7, 2024

fhlavac May 7, 2024

Hyperkid123 May 7, 2024

Hyperkid123 May 7, 2024

fhlavac May 7, 2024 •

edited

Loading

Hyperkid123 May 7, 2024

Hyperkid123 May 7, 2024

Hyperkid123 May 7, 2024

fhlavac commented May 7, 2024

fhlavac commented May 13, 2024

Hyperkid123 left a comment

github-actions bot commented May 14, 2024

github-actions bot commented Jul 31, 2024


		A blank example of the data view layout.
		You can make use of the predefined layout components to display a default header and footer. See [data view toolbar](/data-view/data-view-toolbar) for more information

		@@ -0,0 +1,4 @@
		export interface DataViewPaginationProps {
		page?: number;

Set up DataView layout #5

Set up DataView layout #5

Conversation

fhlavac commented May 6, 2024 • edited Loading

patternfly-build commented May 6, 2024 • edited Loading

fhlavac commented May 6, 2024 • edited Loading

fhlavac May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolethoen commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac May 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac commented May 7, 2024

fhlavac commented May 13, 2024

Hyperkid123 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 14, 2024

github-actions bot commented Jul 31, 2024

fhlavac commented May 6, 2024 •

edited

Loading

patternfly-build commented May 6, 2024 •

edited

Loading

fhlavac commented May 6, 2024 •

edited

Loading

fhlavac May 6, 2024 •

edited

Loading

fhlavac May 7, 2024 •

edited

Loading