patternfly / training-scenarios

PatternFly Developer Training
https://www.patternfly.org/v4/documentation/react/overview/training
8 stars 26 forks source link

Toolbar content review #255

Closed christiemolloy closed 4 years ago

christiemolloy commented 4 years ago

@abigaeljamie helped to provide a review of content. Notes are below:

Change this: In this tutorial, build a toolbar filter and attach it to a table. Toolbars allow users to manage and manipulate a data set. For example, data can be presented in any valid presentation, a table, a list, or a data visualization (chart). The PatternFly Toolbar component is a flexible layout system that accommodates a variety of configurations adapting to numerous needs.

To this: In this tutorial, build a toolbar filter and attach it to a table.

Toolbars allow users to manage and manipulate a data set. For example, data can be presented in any valid presentation, a table, a list, or a data visualization (chart). The PatternFly toolbar component is a flexible layout system that accommodates a variety of configurations.

Page 1 of 6: Swap the order here: Toolbars consist of content rows of components. Most toolbars will only be a single row, but each row is required to be housed in a DataToolbarContent component.

Katacoda is setting up a new React application. Begin coding once the server starts and "Welcome to PatternFly" appears on the lower pane.

So that we have this: Katacoda is setting up a new React application. Begin coding once the server starts and "Welcome to PatternFly" appears on the lower pane.

Toolbars consist of content rows of components. Most toolbars will only be a single row, but each row is required to be housed in a DataToolbarContent component.

Page 2 of 6: Change this: 6) Add DataToolbarItems

To this: 6) Add DataToolbarItems.

Change this: Once the preview reloads - it should look like this:

To this: Once the preview reloads, it should look like this:

Page 3 of 6: Change this: A DataToolbarGroup passes an optional prop variant to format particular types of groups of elements such as filter groups, button groups, or icon button groups.

To this: A DataToolbarGroup passes an optional prop variant to format particular types of element groups such as filter groups, button groups, or icon button groups.

Change this: Note: The filters in the DataToolbarGroup with the filter-group variant have no spacing between them. The DataToolbarGroup with the icon-button-group variant is now spaced closer to each other. (awkward wording)

To this: Note: The filters in the DataToolbarGroup with the filter-group variant have no spacing between them. The DataToolbarGroup with the icon-button-group variant is now spaced closer to each other.

Page 4 of 6: Change this: A toggle group is useful for containing filter controls, for example. When the toolbar responds to adapt to a mobile viewport, the contents contained in a toggle group will collapse into an overlay panel that can be toggled by clicking the Filter icon.

To this: A toggle group is useful for containing filter controls, for example. When the toolbar responds to adapt to a mobile viewport, the contents contained in a toggle group will collapse into an overlay panel. This panel can be toggled by clicking the filter icon.

Change this: Hint: Both the DataToolbarItem and DataToolbarGroup to locate are siblings of each other.

To this: Hint: Both the DataToolbarItem and DataToolbarGroup are siblings of each other.

Change this: 2) Wrap the DataToolbarItem and DataToolbarGroup located in step 1, in a DataToolbarToggleGroup<./strong>

To this: 2) Wrap the DataToolbarItem and DataToolbarGroup located in step 1 in a DataToolbarToggleGroup.

Change this: Changing the breakpoint passed to the DataToolbarToggleGroup should change the viewport width at which the DataToolbarToggleGroup toggles between the expanded state and the responsive collapsed state.

To this: Changing the breakpoint passed to the DataToolbarToggleGroup should change the viewport width at which the DataToolbarToggleGroup toggles between the expanded state and the responsive collapsed state. (awkward wording rephrase)

Page 5 of 6: Change this:
Using a DataToolbarFilter requires three properties. First is a managed array of selected filters called chips as strings or ReactNodes. It also requires an onDelete callback function to be executed whenever the user deletes a selected filter chip. Lastly, it requires a categoryName which will be used to label the chip group.

To this: Using a DataToolbarFilter requires three properties: A managed array of selected filters called chips as strings or ReactNodes. An onDelete callback function to be executed whenever the user deletes a selected filter chip. A categoryName to label the chip group.

Change this: a) For the first DataToolbarFilter (containing a text input) add the property chips={filters.name}.

b) For the second DataToolbarFilter (containing the status filter) add the property chips={filters.status}.

c) For the third DataToolbarFilter (containing the risk filter) add the property chips={filters.risk}.

To this: a) For the first DataToolbarFilter (containing a text input), add the property chips={filters.name}.

b) For the second DataToolbarFilter (containing the status filter), add the property chips={filters.status}.

c) For the third DataToolbarFilter (containing the risk filter), add the property chips={filters.risk}.

Change this: a) For the first DataToolbarFilter (containing a text input) add the property categoryName="Name".

b) For the second DataToolbarFilter (containing the status filter) add the property categoryName="Status".

c) For the third DataToolbarFilter (containing the risk filter) add the property categoryName="Risk".

To this: a) For the first DataToolbarFilter (containing a text input), add the property categoryName="Name".

b) For the second DataToolbarFilter (containing the status filter), add the property categoryName="Status".

c) For the third DataToolbarFilter (containing the risk filter), add the property categoryName="Risk".

Change this: a) Pass a clearAllFilters event handler is passed to DataToolbar.

A 'Clear All Filters' action will appear alongside the applied filters chip groups.

b) If a collapseListedFiltersBreakpoint is passed to DataToolbar.

To this: a) Pass a clearAllFilters event handler to DataToolbar.

**A Clear All Filters action will appear alongside the applied filters chip groups.

b) If a collapseListedFiltersBreakpoint is passed to DataToolbar. (awkward wording rephrase)**

Change this: Note: Take some time to play with the toolbar to see how it responds to changes in viewport size and the number of filters applied.

To this: Note: Take some time to experiment with the toolbar to see how it responds to changes in viewport size and the number of filters applied.

Page 6 of 6: This line seems out of place. We should either delete it or put it somewhere more contextual (or add context): “Managing the relationship of a toolbar with its filters to a table or other component is the responsibility of the consumer.”

nicolethoen commented 4 years ago

@abigaeljamie

Katacoda is setting up a new React application. Begin coding once the server starts and "Welcome to PatternFly" appears on the lower pane.

I'm not seeing this text anywhere. Is it only appearing when certain things are loading? I'm not sure I can control them...

Change this: Note: The filters in the DataToolbarGroup with the filter-group variant have no spacing between them. The DataToolbarGroup with the icon-button-group variant is now spaced closer to each other. (awkward wording)

To this: Note: The filters in the DataToolbarGroup with the filter-group variant have no spacing between them. The DataToolbarGroup with the icon-button-group variant is now spaced closer to each other.

I don't see a difference between these two texts.

Change this: Changing the breakpoint passed to the DataToolbarToggleGroup should change the viewport width at which the DataToolbarToggleGroup toggles between the expanded state and the responsive collapsed state.

To this: Changing the breakpoint passed to the DataToolbarToggleGroup should change the viewport width at which the DataToolbarToggleGroup toggles between the expanded state and the responsive collapsed state. (awkward wording rephrase)

I dont see a difference between these two texts.

I also don't see a difference in the changes requested for the two sections with steps a) b) and c) on page 5.

Are you asking me to try to reword them?

nicolethoen commented 4 years ago

Page 6 of 6: This line seems out of place. We should either delete it or put it somewhere more contextual (or add context): “Managing the relationship of a toolbar with its filters to a table or other component is the responsibility of the consumer.”

Does it make more sense if I remove it? Then I can modify the note in step 1 to read:

Note: The relationship a toolbar and its filters has to a table or other component must be managed by the consumer. The DemoTable component is defined in src/components/table.js where the managed list of applied filters manipulates a very small set of data.

abigaeljamie commented 4 years ago

Hi @nicolethoen ! This was all organized in a Google Doc as feedback for our technical writer. @christiemolloy , was the doc shared with George? Because it was regarding writing feedback and awkward wording, he might be the best person to apply all the editorial feedback.

christiemolloy commented 4 years ago

It was suggested that the owners of the module make the changes so that they can test their module to make sure there are no bugs. But any questions here can be addressed by @gheake2