Skip to content

Commit

Permalink
web: show a disabled selection box, so that the tables align (#5529)
Browse files Browse the repository at this point in the history 
* web: show a disabled selection box, so that the tables align

* response to comments
  • Loading branch information
@nicks
nicks authored Feb 23, 2022
1 parent 58ec004 commit fc9287a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 40 deletions.
55 changes: 32 additions & 23 deletions web/src/OverviewTable.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,12 @@ describe("when disable resources feature is enabled, but `showDisabledResources`
})

describe("bulk disable actions", () => {
function allEnabledCheckboxes(el: HTMLElement) {
return Array.from(
el.querySelectorAll(`${SelectionCheckbox}:not(.Mui-disabled)`)
)
}

describe("when disable resources feature is enabled", () => {
let view: TestDataView
let container: HTMLElement
Expand All @@ -744,6 +750,19 @@ describe("bulk disable actions", () => {
)
})

it("renders labels on enabled checkbox", () => {
let els = container.querySelectorAll(
`${SelectionCheckbox}:not(.Mui-disabled)`
)
expect(els[0].getAttribute("aria-label")).toBe("Resource group selection")
expect(els[1].getAttribute("aria-label")).toBe("Select resource")
})

it("renders labels on disabled checkbox", () => {
let el = container.querySelector(`${SelectionCheckbox}.Mui-disabled`)
expect(el!.getAttribute("aria-label")).toBe("Cannot select resource")
})

it("renders the `Select` column", () => {
expect(
container.querySelectorAll(SelectionCheckbox).length
Expand All @@ -768,48 +787,43 @@ describe("bulk disable actions", () => {
name = row.querySelector(Name).textContent
}

const checkbox = row.querySelectorAll(SelectionCheckbox)
const checkbox = allEnabledCheckboxes(row)
actualCheckboxDisplay[name] = checkbox.length === 1
})

expect(actualCheckboxDisplay).toEqual(expectedCheckBoxDisplay)
})

it("selects a resource when checkbox is not checked", () => {
const checkbox = container.querySelectorAll(SelectionCheckbox)[1]
const checkbox = allEnabledCheckboxes(container)[1]
userEvent.click(checkbox.querySelector("input")!)

const checkboxAfterClick =
container.querySelectorAll(SelectionCheckbox)[1]
const checkboxAfterClick = allEnabledCheckboxes(container)[1]
expect(checkboxAfterClick.getAttribute("aria-checked")).toBe("true")
})

it("deselects a resource when checkbox is checked", () => {
const checkbox = container.querySelectorAll(SelectionCheckbox)[1]
const checkbox = allEnabledCheckboxes(container)[1]
expect(checkbox).toBeTruthy()

// Click the checkbox once to get it to a selected state
userEvent.click(checkbox.querySelector("input")!)

const checkboxAfterFirstClick =
container.querySelectorAll(SelectionCheckbox)[1]
const checkboxAfterFirstClick = allEnabledCheckboxes(container)[1]
expect(checkboxAfterFirstClick.getAttribute("aria-checked")).toBe("true")

// Click the checkbox a second time to deselect it
userEvent.click(checkbox.querySelector("input")!)

const checkboxAfterSecondClick =
container.querySelectorAll(SelectionCheckbox)[1]
const checkboxAfterSecondClick = allEnabledCheckboxes(container)[1]
expect(checkboxAfterSecondClick.getAttribute("aria-checked")).toBe(
"false"
)
})

describe("selection checkbox header", () => {
it("displays as unchecked if no resources in the table are checked", () => {
const allCheckboxes = Array.from(
container.querySelectorAll(SelectionCheckbox)
)
const allCheckboxes = allEnabledCheckboxes(container)
let checkbox: any = allCheckboxes[0]
const headerCheckboxCheckedState = checkbox.getAttribute("aria-checked")
const headerCheckboxIndeterminateState = checkbox
Expand All @@ -826,8 +840,7 @@ describe("bulk disable actions", () => {

it("displays as indeterminate if some but not all resources in the table are checked", () => {
// Choose a (random) table row to click and select
const resourceCheckbox: any =
container.querySelectorAll(SelectionCheckbox)[2]
const resourceCheckbox: any = allEnabledCheckboxes(container)[2]
userEvent.click(resourceCheckbox.querySelector("input"))

// Verify that the header checkbox displays as partially selected
Expand All @@ -844,7 +857,7 @@ describe("bulk disable actions", () => {

it("displays as checked if all resources in the table are checked", () => {
// Click all checkboxes for resource rows, skipping the first one (which is the table header row)
Array.from(container.querySelectorAll(SelectionCheckbox))
allEnabledCheckboxes(container)
.slice(1)
.forEach((resourceCheckbox: any) => {
userEvent.click(resourceCheckbox.querySelector("input"))
Expand All @@ -864,13 +877,11 @@ describe("bulk disable actions", () => {

it("selects every resource in the table when checkbox is not checked", () => {
// Click the header checkbox to select it
const headerCheckbox = container.querySelectorAll(SelectionCheckbox)[0]
const headerCheckbox = allEnabledCheckboxes(container)[0]
userEvent.click(headerCheckbox.querySelector("input")!)

// Verify all table resources are now selected
const rowCheckboxesState = Array.from(
container.querySelectorAll(SelectionCheckbox)
)
const rowCheckboxesState = allEnabledCheckboxes(container)
.slice(1)
.map((checkbox: any) => checkbox.getAttribute("aria-checked"))
expect(rowCheckboxesState).toStrictEqual(["true", "true", "true"])
Expand All @@ -886,9 +897,7 @@ describe("bulk disable actions", () => {
userEvent.click(headerCheckboxAfterFirstClick.querySelector("input")!)

// Verify all table resources are now deselected
const rowCheckboxesState = Array.from(
container.querySelectorAll(SelectionCheckbox)
)
const rowCheckboxesState = allEnabledCheckboxes(container)
.slice(1)
.map((checkbox: any) => checkbox.getAttribute("aria-checked"))
expect(rowCheckboxesState).toStrictEqual(["false", "false", "false"])
Expand All @@ -907,7 +916,7 @@ describe("bulk disable actions", () => {
const firstColumnHeaderText =
container.querySelectorAll(ResourceTableRow)[0].innerHTML

expect(container.querySelectorAll(SelectionCheckbox).length).toBe(0)
expect(allEnabledCheckboxes(container).length).toBe(0)
// Expect to see the Starred column first when the selection column isn't present
expect(firstColumnHeaderText.includes("star.svg")).toBe(true)
})
Expand Down
47 changes: 30 additions & 17 deletions web/src/OverviewTableColumns.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ export const SelectionCheckbox = styled(InstrumentedCheckbox)`
&.Mui-checked {
color: ${Color.gray6};
}
&.Mui-disabled {
opacity: 0.25;
cursor: not-allowed;
}
`

const TableHeaderStarIcon = styled(StarSvg)`
Expand Down Expand Up @@ -315,28 +320,32 @@ export function TableUpdateColumn({ row }: CellProps<RowValues>) {
}

export function TableSelectionColumn({ row }: CellProps<RowValues>) {
// Don't allow a row to be selected if it can't be disabled
// This rule can be adjusted when/if there are other bulk actions
if (!row.original.selectable) {
return null
}

const selections = useResourceSelection()
const resourceName = row.original.name
const checked = selections.isSelected(resourceName)

const onChange = (_e: ChangeEvent<HTMLInputElement>) => {
if (!checked) {
selections.select(resourceName)
} else {
selections.deselect(resourceName)
const onChange = useCallback(
(_e: ChangeEvent<HTMLInputElement>) => {
if (!checked) {
selections.select(resourceName)
} else {
selections.deselect(resourceName)
}
},
[checked, selections]
)

const analyticsTags = useMemo(() => {
return {
...row.original.analyticsTags,
type: AnalyticsType.Grid,
}
}
}, [row.original.analyticsTags])

const analyticsTags = {
...row.original.analyticsTags,
type: AnalyticsType.Grid,
}
let disabled = !row.original.selectable
let label = row.original.selectable
? "Select resource"
: "Cannot select resource"

return (
<SelectionCheckbox
Expand All @@ -346,6 +355,8 @@ export function TableSelectionColumn({ row }: CellProps<RowValues>) {
aria-checked={checked}
onChange={onChange}
size="small"
disabled={disabled}
aria-label={label}
/>
)
}
Expand Down Expand Up @@ -677,14 +688,16 @@ const DEFAULT_COLUMNS: Column<RowValues>[] = [
},
]

let ALL_COLUMNS = [RESOURCE_SELECTION_COLUMN, ...DEFAULT_COLUMNS]

export function getTableColumns(features?: Features) {
if (!features) {
return DEFAULT_COLUMNS
}

// If disable resources is enabled, render the selection column
if (features.isEnabled(Flag.DisableResources)) {
return [RESOURCE_SELECTION_COLUMN, ...DEFAULT_COLUMNS]
return ALL_COLUMNS
}

return DEFAULT_COLUMNS
Expand Down

0 comments on commit fc9287a

Please sign in to comment.