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

FW-6190 clear media selection after adding #632

Closed
wants to merge 1 commit into from

Conversation

sahil97
Copy link
Contributor

@sahil97 sahil97 commented Oct 9, 2024

Description of Changes

  • Clear media array after the media has been added to the instance.

Checklist

  • README / documentation has been updated if needed
  • PR title / commit messages contain Jira ticket number if applicable
  • Tests have been updated / created if applicable

Reviewers Should Do The Following:

  • Review the changes here
  • Pull the branch and test locally

Additional Notes

N/A

Copy link

sonarqubecloud bot commented Oct 9, 2024

@@ -82,6 +84,7 @@ const { array, bool, func, node, object, oneOf } = PropTypes

AddMediaModalWrapper.propTypes = {
selectedMedia: array,
setSelectedMedia: func,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should expose both selectedMedia and setSelectedMedia as parameters, because it introduces a confusing duplication about how to set the selected media.

@@ -26,6 +27,7 @@ function AddMediaModalWrapper({
// allowing user to attach the selected/uploaded files to the document.
const handleOnClick = () => {
if (tabHasSelectedItems) updateSavedMedia(selectedMedia)
setSelectedMedia([])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a bigger problem here than how we handle switching tabs. It appears that the selector is not distinguishing between the full list of selected items (the value of the form control) and the list of items that are currently selected in the modal. Looks like this affects the EntryArrayField as well.

I'm going to need to think about this a bit to figure out the best place to adjust this.

@sahil97 sahil97 closed this Oct 23, 2024
@sahil97
Copy link
Contributor Author

sahil97 commented Oct 23, 2024

Solved in #647.

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.

2 participants