Skip to content

react: Fix react dnd multiple HTML5backends bug#509

Draft
JohannesLares wants to merge 1 commit intoinveniosoftware:masterfrom
JohannesLares:dnd-fix
Draft

react: Fix react dnd multiple HTML5backends bug#509
JohannesLares wants to merge 1 commit intoinveniosoftware:masterfrom
JohannesLares:dnd-fix

Conversation

@JohannesLares
Copy link

React DnD should not use multiple HTML5 backends at the same time. Create one DnD manager, that is used InvenioRDM wide.

❤️ Thank you for your contribution!

Description

Please describe briefly your pull request.

Modified React DnD functionality to have a single html5 backend compared to previous multiple html5 backends. React DnD does not support multiple html5 backends at once (check for example: react-dnd/react-dnd#3119 )

Note! This PR is related to invenio-rdm-records PR: inveniosoftware/invenio-rdm-records#2138

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Frontend

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

React DnD should not use multiple HTML5 backends at the same time.
Create one DnD manager, that is used InvenioRDM wide.
import PropTypes from "prop-types";
import { FieldArray, getIn } from "formik";
import { HTML5Backend } from "react-dnd-html5-backend";
import { manager } from "@js/invenio_rdm_records";
Copy link
Contributor

Choose a reason for hiding this comment

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

i will not comment on the idea itself, because i am not into it at the moment, but here we have dependency issue. normally invenio-vocabulary is a dependency of invenio-rdm-records. reversing that now in the assets would create a circular dependency which can be complicated in the future. would it be possible to create the base manager somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

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

you could do what you did in invenio-rdm-records here and use it then in invenio-rdm-records

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the reply, I did not notice that dependency issue. I'll swap the dependencies around regarding this change.

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

Comments