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

Clean up the docs #152

Merged
merged 8 commits into from
Dec 10, 2024
Merged

Clean up the docs #152

merged 8 commits into from
Dec 10, 2024

Conversation

fhlavac
Copy link
Collaborator

@fhlavac fhlavac commented Dec 3, 2024

closes #22
RHCLOUD-36308

Before:
image

Now:
image

@patternfly-build
Copy link

patternfly-build commented Dec 3, 2024

Copy link

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

I only made it through a small portion of the docs, but just wanted to comment these suggestions so that they don't get lost. Will continue reviewing tomorrow!

---
section: extensions
subsection: Data view
id: Data view
Copy link

Choose a reason for hiding this comment

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

Suggested change
id: Data view
id: Overview
title: Data view overview

What if we change the nav label to just be "Overview" and then expand in the page title to be "Data view overview"? We made similar changes to our ChatBot extension docs, and this would align well with those if you think it sounds okay

Comment on lines 28 to 34
---

**Note:** Data view lives in its own package [`@patternfly/react-data-view`](https://www.npmjs.com/package/@patternfly/react-data-view)

If you notice a bug, or if you have a suggestion for improving the data view extension or its documentation, please file an issue in [the react-data-view repository](https://github.com/patternfly/react-data-view/issues). Before doing so, please make sure there is not already a pre-existing issue.

---
Copy link

Choose a reason for hiding this comment

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

Could we move this note to line 21 to be the first text on the page?

import { useDataViewSelection, useDataViewSort } from '@patternfly/react-data-view/dist/dynamic/Hooks';
import { DataView, DataViewState } from '@patternfly/react-data-view/dist/dynamic/DataView';

## Data view table
Copy link

Choose a reason for hiding this comment

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

Suggested change
## Data view table

What if we let the page title serve this purpose instead?

Comment on lines 24 to 26
The **data view** extension helps you display datasets in organized layouts containing data representation and toolbars allowing interactions like selection or pagination.

Sub-components for displaying the data (card view, table) and toolbars (top and bottom) are always passed as `children` to the `DataView` component.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The **data view** extension helps you display datasets in organized layouts containing data representation and toolbars allowing interactions like selection or pagination.
Sub-components for displaying the data (card view, table) and toolbars (top and bottom) are always passed as `children` to the `DataView` component.
The **data view** extension enables you to present data in an organized layout, with a toolbar that supports interactions like selection and pagination.

What if we move the second sentence to the layout section? I'll add it as a suggestion

Comment on lines 36 to 39
### Layout

Data view is expected to consist of header, data representation part and footer stacked below each other. The layout is implemented using PatternFly [stack](/layouts/stack).

Copy link

Choose a reason for hiding this comment

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

Suggested change
### Layout
Data view is expected to consist of header, data representation part and footer stacked below each other. The layout is implemented using PatternFly [stack](/layouts/stack).
### Layout
A data view should contain a header, the data representation (like a card view or table), and a footer. These parts are organized in a [stack layout](/layouts/stack).
The data view toolbars and sub-components that display the data (like a card view or table) are always passed as `children` to the `<DataView>` component.

Comment on lines 46 to 50
The extension's modular architecture lets you efficiently create consistent data views by using predefined sub-components and hooks or defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.

For the toolbar, you can make use of the predefined `DataViewToolbar` component, which extends the PatternFly [toolbar](/components/toolbar) with the most common use cases. For more details, please refer to the [Toolbar](/extensions/data-view/toolbar) section. In case it does not fit your needs, you can also use your custom toolbar component.

Data can be presented using the predefined `DataViewTable` component, which is an abstraction above the PatternFly [table](/components/table). For more details, please refer to the [Table](/extensions/data-view/table) docs section. In the near future, we are also planning to introduce a predefined Card view component. If you have more specific needs to display data, you can pass your custom implementation as a `DataView` child.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The extension's modular architecture lets you efficiently create consistent data views by using predefined sub-components and hooks or defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.
For the toolbar, you can make use of the predefined `DataViewToolbar` component, which extends the PatternFly [toolbar](/components/toolbar) with the most common use cases. For more details, please refer to the [Toolbar](/extensions/data-view/toolbar) section. In case it does not fit your needs, you can also use your custom toolbar component.
Data can be presented using the predefined `DataViewTable` component, which is an abstraction above the PatternFly [table](/components/table). For more details, please refer to the [Table](/extensions/data-view/table) docs section. In the near future, we are also planning to introduce a predefined Card view component. If you have more specific needs to display data, you can pass your custom implementation as a `DataView` child.
The extension's modular architecture lets you efficiently create consistent data views, by either using predefined sub-components and hooks, or by defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.
The `<DataViewToolbar>` component extends the [PatternFly toolbar](/components/toolbar) to support the most common needs. For more details, refer to the [data view toolbar](/extensions/data-view/toolbar) examples. You can also use a custom toolbar component if needed for your use case.
Data can be presented using the predefined `<DataViewTable>` component, which is an abstraction above the [PatternFly table](/components/table). For more details, refer to the [data view table](/extensions/data-view/table) examples. If you have more specific data display needs, you can pass a custom implementation as a `<DataView>` child. In the near future, we are also planning to introduce a predefined card view component.


### Data view context

The **data view internal context** provides shared state to all sub-components. It lives inside the `DataView` component to store callbacks for the data selection (`onSelect`, `isSelected`, `isSelectDisabled`), internally computed `isSelectable` flag based on selection callbacks passed, and `activeState` of the data view (loading, error, etc.). Its values are set up through props of the `DataView` component.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The **data view internal context** provides shared state to all sub-components. It lives inside the `DataView` component to store callbacks for the data selection (`onSelect`, `isSelected`, `isSelectDisabled`), internally computed `isSelectable` flag based on selection callbacks passed, and `activeState` of the data view (loading, error, etc.). Its values are set up through props of the `DataView` component.
The **data view internal context** provides shared state to all sub-components. It lives inside the `DataView` component to store callbacks for the data selection (`onSelect`, `isSelected`, `isSelectDisabled`), internally computed `isSelectable` flag based on selection callbacks passed, and `activeState` of the data view (loading, error, etc.). Its values are set up through props of the `DataView` component.

Maybe it's just my lack of familiarity with the extension, but I'm struggling to understand this section or the value that this info adds. Is this just explaining how the DataView props are created? Do any of the examples demonstrate this? For me, this is a confusing section, but I'm also not a dev, so I know I'm not the target audience 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, these are unnecessary details. Thank you

Comment on lines 66 to 68
The **data view events context** provides a way of listening to the data view events from the outside of the component through the `DataViewEventsContext`.

In order to give your components an access to the shared context, wrap them and your data view with the `DataViewEventsProvider`.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The **data view events context** provides a way of listening to the data view events from the outside of the component through the `DataViewEventsContext`.
In order to give your components an access to the shared context, wrap them and your data view with the `DataViewEventsProvider`.
The `<DataViewEventsContext>` provides a method of listening to the data view events from the outside of the component.
In order to give your other UI components access to the shared context, wrap them and your data view with the `<DataViewEventsProvider>`.

I'm assuming "your components" means other components outside of the data view, but let me know if I'm wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, components outside of the data view

@fhlavac
Copy link
Collaborator Author

fhlavac commented Dec 5, 2024

@edonehoo thank you for the great comments! I've just pushed the updated version

Copy link

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

Left a bunch of comments/suggestions. GitHub was giving me trouble, so I apologize in advance if this is hard to parse through. Feel free to shoot me a message with any questions!

Also, you can ignore any suggestions that violate any requirements for the docs framework. I'm not sure what's optional vs required


---

The **data view** extension helps you display datasets in organized layouts containing data representation and toolbars allowing interactions like selection or pagination.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The **data view** extension helps you display datasets in organized layouts containing data representation and toolbars allowing interactions like selection or pagination.
The **data view** extension enables you to display datasets in organized layouts, with data representations and interactive toolbars for actions like selection and pagination.


### Layout

A data view should contain a header, the data representation (like a card view or table), and a footer. These parts are organized in a [stack layout](/layouts/stack).
Copy link

Choose a reason for hiding this comment

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

Suggested change
A data view should contain a header, the data representation (like a card view or table), and a footer. These parts are organized in a [stack layout](/layouts/stack).
A data view should contain a header, the data representation, and a footer. These parts are organized in a [stack layout](/layouts/stack).

since we repeat this in the following sentence


### Modularity

The extension's modular architecture lets you efficiently create consistent data views, by either using predefined sub-components and hooks, or by defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The extension's modular architecture lets you efficiently create consistent data views, by either using predefined sub-components and hooks, or by defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.
The extension's modular architecture lets you efficiently create consistent data views, either by using predefined sub-components and hooks or by defining your own. You can choose the tools that suit your needs and easily replace any part with a custom implementation.

Comment on lines 53 to 57
## Advanced concepts

This section contains advanced features related to the `<DataView>` wrapping component and information to better understand how the data view works under the hood.

### Events context
Copy link

Choose a reason for hiding this comment

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

Suggested change
## Advanced concepts
This section contains advanced features related to the `<DataView>` wrapping component and information to better understand how the data view works under the hood.
### Events context
## Events context

since we removed the other section, could we bump up the events context up a level maybe?

Comment on lines 59 to 61
The `<DataViewEventsContext>` provides a method of listening to the data view events from the outside of the component.

In order to give your other UI components access to the shared context, wrap them and your data view with the `<DataViewEventsProvider>`.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The `<DataViewEventsContext>` provides a method of listening to the data view events from the outside of the component.
In order to give your other UI components access to the shared context, wrap them and your data view with the `<DataViewEventsProvider>`.
The `<DataViewEventsContext>` component is an advanced feature that enables external listening of data view events. In order to share data view context with your other UI components, both your components and your data view should be wrapped with the `<DataViewEventsProvider>`. This is demonstrated in the following example.

@@ -62,7 +87,7 @@ This example uses the URL for persisting the pagination state.

```

# Selection
## Selection
Allows to select data records inside the data view and show the selection state.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Allows to select data records inside the data view and show the selection state.
To allow users to select data records inside the data view, add selection support that displays a row's selection state.

@@ -62,7 +87,7 @@ This example uses the URL for persisting the pagination state.

```

# Selection
## Selection
Allows to select data records inside the data view and show the selection state.

### Toolbar usage
Copy link

Choose a reason for hiding this comment

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

Suggested change
### Toolbar usage

@@ -89,7 +114,7 @@ The `useDataViewSelection` hook manages the selection state of the data view.

Copy link

Choose a reason for hiding this comment

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

again, I can't make GH native suggestions for lines 94-113:

The data view toolbar can display a bulk selection component using bulkSelect. You can make use of a predefined component group extension bulk select component.

Selection state

The useDataViewSelection hook manages the selection state of the data viwe.

Initial values:

  • Optional initialSelected array of record's identifiers selected by default
  • matchOption function to check if given record is selected
    • When no matchOption is passed, the Array.prototype.includes() operation is performed on the selected array.
      Return values:
  • selected array of currently selected records
  • isSelected function returning the selection state for given record
  • onSelect callback to modify the selection state and accepting isSelecting flag indicating if records are changing to selected or deselected and items containing affected records

@@ -89,7 +114,7 @@ The `useDataViewSelection` hook manages the selection state of the data view.

```

# Filters
## Filters
Enables filtering of data records in the data view and displays the applied filter chips.
Copy link

Choose a reason for hiding this comment

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

Suggested change
Enables filtering of data records in the data view and displays the applied filter chips.
To allow users to filter data records in the data view, add filtering support that displays the applied filter chips.

@@ -89,7 +114,7 @@ The `useDataViewSelection` hook manages the selection state of the data view.

```

# Filters
## Filters
Enables filtering of data records in the data view and displays the applied filter chips.

### Toolbar usage
Copy link

Choose a reason for hiding this comment

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

For the next section:

The data view toolbar can include a set of filters by passing a React node to the filters property. You can use the predefined components <DataViewFilters>, <DataViewTextFilter>, and <DataViewCheckboxFilter> to customize and handle filtering directly in the toolbar. The <DataViewFilters> component is a wrapper that allows conditional filtering using multiple attributes. If you need just a single filter, you can use <DataViewTextFilter>, <DataViewCheckboxFilter>, or a different filter component alone. Props of these filter components are listed at the bottom of this page.

You can either pass a value and onChange event to every filter separately, or you can pass values and onChange to the <DataViewFilters> wrapper, which make them available to its children. Props directly passed to child filters have a higher priority than the "inherited" ones.

Filters state

The useDataViewFilters hook manages the filter state of the data view. It allows you to define default filter values, synchronize filter state with URL parameters, and handle filter changes efficiently.

Initial values:

  • initialFilters object with default filter values (if the filter param allows multiple values, pass an array)
  • Optional searchParams object for managing URL-based filter state
  • Optional setSearchParams function to update the URL when filters are modified

The useDataViewFilters hook works well with the React Router library to support URL-based filtering. Alternatively, you can manage filter state in the URL using URLSearchParams and window.history.pushState APIs, or other routing libraries. If no URL parameters are provided, the filter state is managed internally.

Return values:

  • filters object representing the current filter values
  • onSetFilters function to update the filter state
  • clearAllFilters function to reset all filters to their initial values

@fhlavac
Copy link
Collaborator Author

fhlavac commented Dec 9, 2024

@edonehoo I tried to add the requested changes. Please let me know if something needs to be further improved. Thank you for all your comments!

@fhlavac fhlavac requested a review from edonehoo December 9, 2024 13:55
Copy link

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

just a few small formatting suggestions, but overall I think this content is in a good place/ready to go live! Nothing big to hold up the release

## Selection
To allow users to select data records inside the data view, add selection support that displays a row's selection state.

The data view toolbar can display a bulk selection component using bulkSelect. You can make use of a predefined [component group extension bulk select](/extensions/component-groups/bulk-select) component.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The data view toolbar can display a bulk selection component using bulkSelect. You can make use of a predefined [component group extension bulk select](/extensions/component-groups/bulk-select) component.
The data view toolbar can display a bulk selection component by using the predefined [component group extension bulk select](/extensions/component-groups/bulk-select) component.

Comment on lines 101 to 102

*When no `matchOption` is passed, the `Array.prototype.includes()` operation is performed on the `selected` array.*
Copy link

Choose a reason for hiding this comment

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

Suggested change
*When no `matchOption` is passed, the `Array.prototype.includes()` operation is performed on the `selected` array.*
- When no `matchOption` is passed, the `Array.prototype.includes()` operation is performed on the `selected` array.

I would indent this under the matchOption line if you can

## Filters
To allow users to filter data records in the data view, add filtering support that displays the applied filter chips.

The data view toolbar can include a set of filters by passing a React node to the `filters` property. You can use the predefined components `<DataViewFilters>`, `<DataViewTextFilter>`, and `<DataViewCheckboxFilter>` to customize and handle filtering directly in the toolbar. The `<DataViewFilters>` component is a wrapper that allows conditional filtering using multiple attributes. If you need just a single filter, you can use `<DataViewTextFilter>`, `<DataViewCheckboxFilter>`, or a different filter component alone. Props of these filter components are listed at the bottom of this page.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The data view toolbar can include a set of filters by passing a React node to the `filters` property. You can use the predefined components `<DataViewFilters>`, `<DataViewTextFilter>`, and `<DataViewCheckboxFilter>` to customize and handle filtering directly in the toolbar. The `<DataViewFilters>` component is a wrapper that allows conditional filtering using multiple attributes. If you need just a single filter, you can use `<DataViewTextFilter>`, `<DataViewCheckboxFilter>`, or a different filter component alone. Props of these filter components are listed at the bottom of this page.
The data view toolbar can include a set of filters by passing a React node to the `filters` property. You can use the predefined components `<DataViewFilters>`, `<DataViewTextFilter>`, and `<DataViewCheckboxFilter>` to customize and handle filtering directly in the toolbar. The `<DataViewFilters>` component is a wrapper that allows conditional filtering using multiple attributes. If you need just a single filter, you can use `<DataViewTextFilter>`, `<DataViewCheckboxFilter>`, or a different filter component alone. Props of these filter components are listed in the [props section of this page](#props).

we just typically avoid words like below/above for accessibility purposes. so however you'd like to adjust that!

- Optional `searchParams` object
- Optional `setSearchParams` function

While the hook works seamlessly with the React Router library, you do not need to use it to take advantage of URL persistence. The `searchParams` and `setSearchParams` props can be managed using native browser APIs (`URLSearchParams` and `window.history.pushState`) or any other routing library of your choice. If you don't pass these two props, the pagination state will be stored internally without the URL usage.
Copy link

Choose a reason for hiding this comment

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

Suggested change
While the hook works seamlessly with the React Router library, you do not need to use it to take advantage of URL persistence. The `searchParams` and `setSearchParams` props can be managed using native browser APIs (`URLSearchParams` and `window.history.pushState`) or any other routing library of your choice. If you don't pass these two props, the pagination state will be stored internally without the URL usage.
While the hook works seamlessly with the [React Router](https://reactrouter.com/) library, you do not need to use it to take advantage of URL persistence. The `searchParams` and `setSearchParams` props can be managed using native browser APIs (`URLSearchParams` and `window.history.pushState`) or any other routing library of your choice. If you don't pass these two props, the pagination state will be stored internally without the URL usage.

- Optional `searchParams` object for managing URL-based filter state
- Optional `setSearchParams` function to update the URL when filters are modified

The `useDataViewFilters` hook works well with the React Router library to support URL-based filtering. Alternatively, you can manage the filter state in the URL using `URLSearchParams` and `window.history.pushState` APIs, or other routing libraries. If no URL parameters are provided, the filter state is managed internally.
Copy link

Choose a reason for hiding this comment

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

Suggested change
The `useDataViewFilters` hook works well with the React Router library to support URL-based filtering. Alternatively, you can manage the filter state in the URL using `URLSearchParams` and `window.history.pushState` APIs, or other routing libraries. If no URL parameters are provided, the filter state is managed internally.
The `useDataViewFilters` hook works well with the [React Router](https://reactrouter.com/) library to support URL-based filtering. Alternatively, you can manage the filter state in the URL using `URLSearchParams` and `window.history.pushState` APIs, or other routing libraries. If no URL parameters are provided, the filter state is managed internally.

Comment on lines 77 to 80
- Current `page` number
- `onSetPage` to modify current page
- Items `perPage` value
- `onPerPageSelect` to modify per page value
Copy link

Choose a reason for hiding this comment

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

Suggested change
- Current `page` number
- `onSetPage` to modify current page
- Items `perPage` value
- `onPerPageSelect` to modify per page value
- Current `page` number.
- `onSetPage` to modify current page.
- Items `perPage` value.
- `onPerPageSelect` to modify per page value.

Comment on lines 99 to 100
- Optional `initialSelected` array of record's identifiers selected by default
- `matchOption` function to check if given record is selected
Copy link

Choose a reason for hiding this comment

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

Suggested change
- Optional `initialSelected` array of record's identifiers selected by default
- `matchOption` function to check if given record is selected
- Optional `initialSelected` array of record's identifiers selected by default.
- `matchOption` function to check if given record is selected.

Comment on lines 105 to 107
- `selected` array of currently selected records
- `isSelected` function returning the selection state for given record
- `onSelect` callback to modify the selection state and accepting `isSelecting` flag indicating if records are changing to selected or deselected and `items` containing affected records
Copy link

Choose a reason for hiding this comment

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

Suggested change
- `selected` array of currently selected records
- `isSelected` function returning the selection state for given record
- `onSelect` callback to modify the selection state and accepting `isSelecting` flag indicating if records are changing to selected or deselected and `items` containing affected records
- `selected` array of currently selected records.
- `isSelected` function returning the selection state for given record.
- `onSelect` callback to modify the selection state and accepting `isSelecting` flag indicating if records are changing to selected or deselected and `items` containing affected records.

Comment on lines 127 to 129
- `initialFilters` object with default filter values (if the filter param allows multiple values, pass an array)
- Optional `searchParams` object for managing URL-based filter state
- Optional `setSearchParams` function to update the URL when filters are modified
Copy link

Choose a reason for hiding this comment

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

Suggested change
- `initialFilters` object with default filter values (if the filter param allows multiple values, pass an array)
- Optional `searchParams` object for managing URL-based filter state
- Optional `setSearchParams` function to update the URL when filters are modified
- `initialFilters` object with default filter values (if the filter param allows multiple values, pass an array).
- Optional `searchParams` object for managing URL-based filter state.
- Optional `setSearchParams` function to update the URL when filters are modified.

Comment on lines 134 to 136
- `filters` object representing the current filter values
- `onSetFilters` function to update the filter state
- `clearAllFilters` function to reset all filters to their initial values
Copy link

Choose a reason for hiding this comment

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

Suggested change
- `filters` object representing the current filter values
- `onSetFilters` function to update the filter state
- `clearAllFilters` function to reset all filters to their initial values
- `filters` object representing the current filter values.
- `onSetFilters` function to update the filter state.
- `clearAllFilters` function to reset all filters to their initial values.

@fhlavac fhlavac merged commit a144ea3 into patternfly:v5 Dec 10, 2024
7 checks passed
@fhlavac fhlavac deleted the clean-docs branch December 10, 2024 14:12
Copy link

🎉 This PR is included in version 5.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants