Shop channel fix selectable index table #4368
Closed
Conversation
This file contains hidden or 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
|
👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. |
Within the IndexProvider the value is accepted as a prop but isn't being used, when it should be. There's already an existing identifier "selectable" which can be renamed as "actionable" because it indicates whether or not actions can be made (are there bulk actions available). Similarly in the useBulkSelectionData hook there's an existing identifier "selectable" where it means "does it currently have any selections. It's a temporary variable created to indicate whether or not any selections have been made, so we rename it to "hasSelections" even though the scope doesn't conflict - just to make it clearer.
This was previously always the case, and when the selectable prop is not provided we would still expect it to default to true, because the main behaviour of this component is for use as a selectable index. This provides backwards compatibility, while still supporting explicitly passing this value as true/false.
Simplifies the closure function because it no longer has multiple return signatures (returning either JSX or JSX[], depending on whether or not its the first element). Splits out a second one that's specifically for rendering the checkbox, and then explicitly calling that and pushing it into the array when selectable.
brendanatshopify
force-pushed
the
shop-channel-fix-selectable-index-table
branch
from
96a18d5 to
67240ff
Compare
Contributor
Author
|
Closing in favor of #4376 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
WHY are these changes introduced?
Fixes #4076
WHAT is this pull request doing?
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
Copy-paste this code in
playground/Playground.tsx:🎩 checklist
README.mdwith documentation changes