surveyjs / survey-creator

Scalable open-source survey software to generate dynamic JSON-driven forms within your JavaScript application. The form builder features a drag-and-drop UI, CSS Theme Editor, and GUI for conditional logic and form branching.
https://surveyjs.io/open-source
Other
896 stars 370 forks source link

[V2] Add dragged element as child of survey creator root #2236

Open SamMousa opened 2 years ago

SamMousa commented 2 years ago

Are you requesting a feature, reporting a bug or ask a question?

Bug / feature

What is the current behavior?

Dragging an element causes the dragged element to be appended to the end of the body.

What is the expected behavior?

Ideally all DOM elements should be children of the survey creator root element. We embed inside a page that has a different setting for border-box for example. We can fix this in the survey-creator root element with some CSS, but then we have to apply extra fixes for those elements that are dynamically generated.

Since the element is attached to the cursor it is already absolutely positioned so there is no requirement for having it outside the survey creator's root element.

Provide the test code and the tested page URL (if applicable)

Tested page URL: https://surveyjs.io/create-survey-v2

dmitry-kurmanov commented 2 years ago

Sounds reasonable. I will investigate how we could implement the proposal and come back asap.

SamMousa commented 2 years ago

Stupid question: where does the source for v2 live? If I knew I could have created a PR instead of an issue ;-)

SamMousa commented 2 years ago

I've looked a bit into how drag&drop is implemented, and it seems the "old-school" approach of using javascript with mousemove to track the mouse is used. This is very laggy, especially if you move the mouse faster. I suspect the reason for not using the native drag & drop system is the fact that you don't want to drag away the toolbox item, but instead some instantiated version of the question type.

There is an API for creating a preview of a different element.

I've created a fiddle here: https://jsfiddle.net/sammousa/a4b9dx6L/67/

This uses the native drag & drop API.

dmitry-kurmanov commented 2 years ago

Stupid question: where does the source for v2 live? If I knew I could have created a PR instead of an issue ;-)

here: https://github.com/surveyjs/survey-creator/tree/master/packages survey-creator folder is the old one and the others survey-creator-core, survey-creator-react, survey-creator-knockout is for creator v2

I've looked a bit into how drag&drop is implemented, and it seems the "old-school" approach

Yes you are right we are using the "old-school" approach and DnD emulation and we choose that approach because setDragImage doesn't allow us to implement all UX requirements. There were some issues I faced with :

  1. Make the dragImage opacity = 1. It is important for our UX.
  2. Change the dragImage content while "dragOver". For example ranking question. I think that setDragImage is designed mostly for the images only not for the DOM nodes. But maybe I am wrong.
  3. Touch devices support (still in progress)
SamMousa commented 2 years ago

I've solved number 2 in my fiddle; setDragImage can draw any node in the DOM.

dmitry-kurmanov commented 2 years ago

Could you please create an example with changing content of the "dragImage" cloned node while dragging? As I understand you setup that content on dragStart for now. Please check ranking question numbers:

image

SamMousa commented 2 years ago

Ah you mean changing the image based on the drop target it is over? Yeah, that's not possible at the moment. Note that you shouldn't limit yourself to 1 kind of drag; ie use the manual one in small ranges for ordering items, but use the proper one for adding questions to a survey.

Currently the creator is just very laggy when doing drag & drop from the toolbox. Personally I'd rather have smooth drag & drop, and the UX can be improved by showing contextual information in the drop target instead of on the cursor.

dmitry-kurmanov commented 2 years ago

here the separate issue for that: https://github.com/surveyjs/survey-creator/issues/2243

dmitry-kurmanov commented 2 years ago

we've added dragged element as child of survey creator root via the https://github.com/surveyjs/survey-library/pull/3664. It will be available in the next minor release.

dmitry-kurmanov commented 2 years ago

@SamMousa unfortunately after the review of my pull request the team decided not to merge the changes for now. We are working hard on creatorV2 design and for now focusing on it.

We added that issue to our project's TODO and we'll return to it after the Creator V2 release. Thanks for the proposal.

SamMousa commented 2 years ago

No problem, thanks for the update

SamMousa commented 9 months ago

This issue makes theming the creator hard if you want to apply your variables to the creator and not globally. (ignore the padding issue, it's unrelated)

image

image