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

EDX-1352: batch select sheet list documents #6713

Merged
merged 130 commits into from
May 22, 2024

Conversation

jordanl17
Copy link
Member

@jordanl17 jordanl17 commented May 17, 2024

Description

EDX-1352: allowing multiselection of rows using shift+click

What to review

The dir structure has been changed so that all sheetList components and files are inside a separated dir within documentList and EX has been added a code owner.

Testing

Tests have been added to the row selection component - DocumentSheetListSelect to test around shift click functionality

Notes for release

No relevant notes as this feature is not currently exposed to users

jordanl17 and others added 30 commits May 7, 2024 11:54
* fix(studio): adding tooltip to read-only bool inputs

* fix(studio): testing for tooltip on boolean read-only inputs

* fix(studio): removing memoisation as it was useless
Co-authored-by: juice49 <1454914+juice49@users.noreply.github.com>
* feat: add canHandleIntent to S.component

* fix: properly type canHandleIntent

* Update packages/sanity/src/structure/structureBuilder/Component.ts

Co-authored-by: Ash <ash@sanity.io>

---------

Co-authored-by: Ash <ash@sanity.io>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* feat(vision): add download as json/csv buttons

* fix(vision): use blob urls for downloads (#6213)

* fix(vision): use Translate component to avoid splitting i18n strings

* fix(vision): clean up i18n resources for result saving feature

---------

Co-authored-by: Espen Hovlandsdal <espen@hovlandsdal.com>
… editing form (#6610)

This will prevent any input calling element.onFocus() on any opened block or inline-object
inside the PT-input, as that will close the editing modal for them (through DocumentPaneProvider)
* feat(structure): Rendering sheet list layout option in test-studio

* feat(structure): branching the rendering of different document list panes

* feat(structure): renaming of generic pane components

* feat(structure): fixing typing for new sheetList

* feat(structure): resolving a default export to named

* feat(structure): testing sheet view pane display logcic

* fix(structure): resolving testing for useStructureTool
* test(core): add tests for document list sort and display

* test(structure): add test for inspect dialog

* test(core): add tests for saved searches
* test(core): fixes flaky test with document publish

* test(core): use more realistic fix for flaky test
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* chore(deps): update dependency styled-components to ^6.1.10

* chore: update test snapshot

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Cody Olsen <stipsan@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…6584)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
expect(selectProps.table.options.meta?.setSelectedAnchor).toHaveBeenCalledWith(null)
})

it('disables the checkbox when row is not selectable', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reason that a row is not selectable? For a11y, it would be best to not have elements disabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh totally! Currently there is no reason why a row would be selected, but tanstack table does support this functionality, so I thought I'd add a theoretical test case to ensure that any future disabling. In the future we mihgt disable, perhaps whilst we are running a batch publish and don't want the user change row selection. Added here so it's already tested

indeterminate={info.table.getIsSomeRowsSelected()}
onChange={info.table.getToggleAllPageRowsSelectedHandler()}
/>
{/* eslint-disable-next-line i18next/no-literal-string */}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if selected should be added to the attribute exceptions list

Copy link
Member Author

Choose a reason for hiding this comment

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

This was just some text I'm printing to the DOM so we can make sure that the correct rows are being selected. It's quite likely this would be deleted when we start improving styling

Copy link
Contributor

@jtpetty jtpetty left a comment

Choose a reason for hiding this comment

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

a few minor questions, but 👍 LGTM

Copy link
Contributor

@pedrobonamin pedrobonamin left a comment

Choose a reason for hiding this comment

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

Thanks Jordan, looks good to me. Let's have it merged after the release

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be using the StudioUI components here as well, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

The presentation of this component is just placeholder for now since we are still awaiting designs

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

Successfully merging this pull request may close these issues.