-
Notifications
You must be signed in to change notification settings - Fork 450
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-1397 #6741
Merged
Merged
EDX-1397 #6741
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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 <[email protected]>
* feat: add canHandleIntent to S.component * fix: properly type canHandleIntent * Update packages/sanity/src/structure/structureBuilder/Component.ts Co-authored-by: Ash <[email protected]> --------- Co-authored-by: Ash <[email protected]>
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 <[email protected]>
… 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 <[email protected]>
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great progress, thanks @jordanl17
Minor things I noticed:
- The select inputs are not showing the "selected" state, it's missing the
border: getBorderStyle(),
style, adding them would be nice. - Also on select, they don't have the
ref
added, so clicking on enter won't focus it as it does with the inputs. - Context wise: The context added to the singleton is called
DocumentSheetListContext
and the provider isSheetListSelectionProvider
it will be great to have both named accordingly, I suggest we go forDocumentSheetListProvider
in case we need to later add more things and not make it scoped toselection
, what do you think?
pedrobonamin
approved these changes
May 24, 2024
rexxars
pushed a commit
that referenced
this pull request
May 24, 2024
Co-authored-by: Pedro Bonamin <[email protected]>
This was referenced Jun 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes EDX-1397: supports copy/pasting across multiple documents on the same field
What to review
At a high level the approach here has been:
DocumentSheetListSelectionProvider
- wraps the entire table and manages the selected and focused states of all cells. Keeps track of user selection, and listens for keyboard events to change the selected cell and selection. Exposes a cell's state throughgetStateByCellId
SheetListCell
- usesgetStateByCellId
to determines it's state. Using the state, the cell conditionally adds/removes event listeners as appropriate, eg. it adds a listener to copy events when the cell is only selected (and so not actually focused)General behaviour has been mimicked from Google Sheets, with some alterations:
https://www.loom.com/share/285d887f551844a692c84179fadd12dd?sid=068cffcf-34bb-4e9e-8996-b37a866a3a0d
Testing
Tests added to
DocumentSheetListPane
as this was the easiest way of setting up the table state (from tanstack) and rendering out all that's needed in order to test keyboard behaviour.Notes for release
No relevant notes, this feature is not currently user facing