Skip to content

Implement read-only view of individual collection page #3636

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.

Sign up for GitHub

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

Merged
dvail merged 3 commits into from
Nov 2, 2022
Merged

Implement read-only view of individual collection page #3636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1aa9524
Implement read-only view of individual collection page
dvail Oct 31, 2022
1c8112b
Fix import
dvail Nov 1, 2022
a8ca9d0
Semantic fixes and text updates
dvail Nov 2, 2022
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
3 changes: 1 addition & 2 deletions ui/apps/platform/src/Components/PatternFly/BacklogListSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
EmptyStateVariant,
Flex,
FormGroup,
Title,
} from '@patternfly/react-core';
import { CubesIcon, MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons';

Expand Down Expand Up @@ -92,7 +91,7 @@ function BacklogTable<Item>({
) : (
<EmptyState variant={EmptyStateVariant.xs}>
<EmptyStateIcon icon={CubesIcon} />
<Title headingLevel="h4">No items remaining</Title>
<p>No items remaining</p>
</EmptyState>
)}
</FormGroup>
Expand Down
81 changes: 67 additions & 14 deletions ui/apps/platform/src/Containers/Collections/CollectionForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import {
DropdownItem,
DropdownSeparator,
DropdownToggle,
EmptyState,
EmptyStateIcon,
EmptyStateVariant,
Flex,
FlexItem,
Form,
Expand All @@ -28,8 +31,10 @@ import {
Text,
TextInput,
Title,
Truncate,
} from '@patternfly/react-core';
import { CaretDownIcon } from '@patternfly/react-icons';
import { CaretDownIcon, CubesIcon } from '@patternfly/react-icons';
import { TableComposable, TableVariant, Tbody, Tr, Td } from '@patternfly/react-table';
import { useFormik } from 'formik';
import * as yup from 'yup';
import isEmpty from 'lodash/isEmpty';
Expand All @@ -46,6 +51,32 @@ import CollectionAttacher from './CollectionAttacher';
import CollectionResults from './CollectionResults';
import { Collection, ScopedResourceSelector, SelectorEntityType } from './types';

function AttachedCollectionTable({ collections }: { collections: CollectionResponse[] }) {
return collections.length > 0 ? (
<TableComposable aria-label="Attached collections" variant={TableVariant.compact}>
<Tbody>
{collections.map(({ name, description }) => (
<Tr key={name}>
<Td dataLabel="Name">
<Button variant="link" className="pf-u-pl-0" isInline>
{name}
</Button>
</Td>
<Td dataLabel="Description">
<Truncate content={description} />
</Td>
</Tr>
))}
</Tbody>
</TableComposable>
) : (
<EmptyState variant={EmptyStateVariant.xs}>
<EmptyStateIcon icon={CubesIcon} />
<p>There are no other collections attached to this collection</p>
</EmptyState>
Copy link
Copy Markdown
Contributor

@pedrottimark pedrottimark Nov 1, 2022

Choose a reason for hiding this comment

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

This is consistent with many other empty state titles. However, a question for PatternFly Developer Office Hours is pro and con for accessibility of h4 element versus p element with the identical style.

In a video for a past day of learning with theme of accessibility, the screen reader user used heading structure of page quite a bit to form a mental model of the information hierarchy (and commented on any skips in heading levels). This page has intuitive hierarchy for h1 and h2 but not so much for this heading.

Copy link
Copy Markdown
Contributor Author

@dvail dvail Nov 2, 2022

Choose a reason for hiding this comment

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

True I agree, especially with the screen reader comment since this bit of text doesn't really communicate hierarchy. The page has h1 and h2, but there isn't anything semantically that makes this a h3, h4, etc. I'll update to a <p> with styles.

);
}

export type CollectionFormProps = {
hasWriteAccessForCollections: boolean;
/* The user's workflow action for this collection */
Expand Down Expand Up @@ -137,6 +168,7 @@ function CollectionForm({
}, [toggleDrawer, useInlineDrawer]);

const pageTitle = action.type === 'create' ? 'Create collection' : values.name;
const isReadOnly = action.type === 'view' || !hasWriteAccessForCollections;

function onEditCollection(id: string) {
history.push({
Expand Down Expand Up @@ -317,6 +349,7 @@ function CollectionForm({
validated={errors.name ? 'error' : 'default'}
onChange={(_, e) => handleChange(e)}
onBlur={handleBlur}
isDisabled={isReadOnly}
/>
</FormGroup>
</FlexItem>
Expand All @@ -328,6 +361,7 @@ function CollectionForm({
value={values.description}
onChange={(_, e) => handleChange(e)}
onBlur={handleBlur}
isDisabled={isReadOnly}
/>
</FormGroup>
</FlexItem>
Expand All @@ -339,19 +373,26 @@ function CollectionForm({
direction={{ default: 'column' }}
spaceItems={{ default: 'spaceItemsMd' }}
>
<Title className="pf-u-mb-xs" headingLevel="h2">
Add new collection rules
<Title
className={isReadOnly ? 'pf-u-mb-md' : 'pf-u-mb-xs'}
headingLevel="h2"
>
Collection rules
</Title>
<p>
Select deployments via rules. You can use regular
expressions (RE2 syntax).
</p>
<Divider className="pf-u-mb-lg" component="div" />
{!isReadOnly && (
<>
<p>
Select deployments via rules. You can use regular
expressions (RE2 syntax).
</p>
</>
)}
<RuleSelector
entityType="Deployment"
scopedResourceSelector={values.resourceSelectors.Deployment}
handleChange={onResourceSelectorChange}
validationErrors={errors.resourceSelectors?.Deployment}
isDisabled={isReadOnly}
/>
<Label
variant="outline"
Expand All @@ -365,6 +406,7 @@ function CollectionForm({
scopedResourceSelector={values.resourceSelectors.Namespace}
handleChange={onResourceSelectorChange}
validationErrors={errors.resourceSelectors?.Namespace}
isDisabled={isReadOnly}
/>
<Label
variant="outline"
Expand All @@ -378,6 +420,7 @@ function CollectionForm({
scopedResourceSelector={values.resourceSelectors.Cluster}
handleChange={onResourceSelectorChange}
validationErrors={errors.resourceSelectors?.Cluster}
isDisabled={isReadOnly}
/>
</Flex>

Expand All @@ -387,13 +430,23 @@ function CollectionForm({
spaceItems={{ default: 'spaceItemsMd' }}
>
<Title className="pf-u-mb-xs" headingLevel="h2">
Attach existing collections
Attached collections
</Title>
<p>Extend this collection by attaching other sets.</p>
<CollectionAttacher
initialEmbeddedCollections={initialEmbeddedCollections}
onSelectionChange={onEmbeddedCollectionsChange}
/>
{isReadOnly ? (
<AttachedCollectionTable
collections={initialEmbeddedCollections}
/>
) : (
<>
<p>Extend this collection by attaching other sets.</p>
<CollectionAttacher
initialEmbeddedCollections={
initialEmbeddedCollections
}
onSelectionChange={onEmbeddedCollectionsChange}
/>
</>
)}
</Flex>
</Flex>
{action.type !== 'view' && (
Expand Down
3 changes: 3 additions & 0 deletions ui/apps/platform/src/Containers/Collections/RuleSelector/AutoCompleteSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type AutoCompleteSelectProps = {
typeAheadAriaLabel?: string;
onChange: (value: string) => void;
validated: ValidatedOptions;
isDisabled: boolean;
};

/* TODO Implement autocompletion */
Expand All @@ -19,6 +20,7 @@ export function AutoCompleteSelect({
typeAheadAriaLabel,
onChange,
validated,
isDisabled,
}: AutoCompleteSelectProps) {
const { isOpen, onToggle, closeSelect } = useSelectToggle();

Expand All @@ -40,6 +42,7 @@ export function AutoCompleteSelect({
onToggle={onToggle}
selections={selectedOption}
onSelect={onSelect}
isDisabled={isDisabled}
/>
</>
);
Expand Down
60 changes: 38 additions & 22 deletions ui/apps/platform/src/Containers/Collections/RuleSelector/ByLabelSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ export type ByLabelSelectorProps = {
scopedResourceSelector: ScopedResourceSelector
) => void;
validationErrors: FormikErrors<ByLabelResourceSelector> | undefined;
isDisabled: boolean;
};

function ByLabelSelector({
entityType,
scopedResourceSelector,
handleChange,
validationErrors,
isDisabled,
}: ByLabelSelectorProps) {
const { keyFor, invalidateIndexKeys } = useIndexKey();
function onChangeLabelKey(resourceSelector: ByLabelResourceSelector, ruleIndex, value) {
Expand Down Expand Up @@ -139,6 +141,7 @@ function ByLabelSelector({
)
}
validated={keyValidation}
isDisabled={isDisabled}
/>
</FormGroup>
<FlexItem
Expand Down Expand Up @@ -188,38 +191,51 @@ function ByLabelSelector({
)
}
validated={valueValidation}
isDisabled={isDisabled}
/>
<Button
variant="plain"
onClick={() =>
onDeleteValue(ruleIndex, valueIndex)
}
>
<TrashIcon
style={{ cursor: 'pointer' }}
color="var(--pf-global--Color--dark-200)"
/>
</Button>
{!isDisabled && (
<Button
variant="plain"
onClick={() =>
onDeleteValue(ruleIndex, valueIndex)
}
>
<TrashIcon
style={{ cursor: 'pointer' }}
color="var(--pf-global--Color--dark-200)"
/>
</Button>
)}
</Flex>
);
})}
</Flex>
<Button
className="pf-u-pl-0 pf-u-pt-md"
variant="link"
onClick={() => onAddLabelValue(ruleIndex)}
>
Add value
</Button>
{!isDisabled && (
<Button
className="pf-u-pl-0 pf-u-pt-md"
variant="link"
onClick={() => onAddLabelValue(ruleIndex)}
>
Add value
</Button>
)}
</FormGroup>
</Flex>
</div>
);
})}
<Divider component="div" className="pf-u-pt-lg" />
<Button className="pf-u-pl-0 pf-u-pt-md" variant="link" onClick={onAddLabelRule}>
Add label rule
</Button>
{!isDisabled && (
<>
<Divider component="div" className="pf-u-pt-lg" />
<Button
className="pf-u-pl-0 pf-u-pt-md"
variant="link"
onClick={onAddLabelRule}
>
Add label rule
</Button>
</>
)}
</>
);
}
Expand Down
29 changes: 18 additions & 11 deletions ui/apps/platform/src/Containers/Collections/RuleSelector/ByNameSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ export type ByNameSelectorProps = {
scopedResourceSelector: ScopedResourceSelector
) => void;
validationErrors: FormikErrors<ByNameResourceSelector> | undefined;
isDisabled: boolean;
};

function ByNameSelector({
entityType,
scopedResourceSelector,
handleChange,
validationErrors,
isDisabled,
}: ByNameSelectorProps) {
const { keyFor, invalidateIndexKeys } = useIndexKey();

Expand Down Expand Up @@ -74,21 +76,26 @@ function ByNameSelector({
? ValidatedOptions.error
: ValidatedOptions.default
}
isDisabled={isDisabled}
/>
<Button variant="plain" onClick={() => onDeleteValue(index)}>
<TrashIcon
aria-label={`Delete ${value}`}
className="pf-u-flex-shrink-1"
style={{ cursor: 'pointer' }}
color="var(--pf-global--Color--dark-200)"
/>
</Button>
{!isDisabled && (
<Button variant="plain" onClick={() => onDeleteValue(index)}>
<TrashIcon
aria-label={`Delete ${value}`}
className="pf-u-flex-shrink-1"
style={{ cursor: 'pointer' }}
color="var(--pf-global--Color--dark-200)"
/>
</Button>
)}
</Flex>
))}
</Flex>
<Button className="pf-u-pl-0 pf-u-pt-md" variant="link" onClick={onAddValue}>
Add value
</Button>
{!isDisabled && (
<Button className="pf-u-pl-0 pf-u-pt-md" variant="link" onClick={onAddValue}>
Add value
</Button>
)}
</FormGroup>
);
}
Expand Down
5 changes: 5 additions & 0 deletions ui/apps/platform/src/Containers/Collections/RuleSelector/RuleSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ export type RuleSelectorProps = {
scopedResourceSelector: ScopedResourceSelector
) => void;
validationErrors: FormikErrors<ScopedResourceSelector> | undefined;
isDisabled?: boolean;
};

function RuleSelector({
entityType,
scopedResourceSelector,
handleChange,
validationErrors,
isDisabled = false,
}: RuleSelectorProps) {
const { isOpen, onToggle, closeSelect } = useSelectToggle();
const pluralEntity = pluralize(entityType);
Expand Down Expand Up @@ -84,6 +86,7 @@ function RuleSelector({
onToggle={onToggle}
selections={selection}
onSelect={onRuleOptionSelect}
isDisabled={isDisabled}
>
<SelectOption value="All">All {pluralEntity.toLowerCase()}</SelectOption>
<SelectOption value="ByName">{pluralEntity} with names matching</SelectOption>
Expand All @@ -96,6 +99,7 @@ function RuleSelector({
scopedResourceSelector={scopedResourceSelector}
handleChange={handleChange}
validationErrors={validationErrors}
isDisabled={isDisabled}
/>
)}

Expand All @@ -105,6 +109,7 @@ function RuleSelector({
scopedResourceSelector={scopedResourceSelector}
handleChange={handleChange}
validationErrors={validationErrors}
isDisabled={isDisabled}
/>
)}
</div>
Expand Down
Loading