[GSoC2024] Implemented drag and drop feature #7624

KrishavRajSingh · 2024-03-17T14:17:16Z

Added the feature of dragging attributes

Motivation and context

resolves #7370

Checklist

[*] I submit my changes into the develop branch
[*] I have created a changelog fragment
I have updated the documentation accordingly
I have added tests to cover my changes
[*] I have linked related issues (see GitHub docs)
I have increased versions of npm packages if it is necessary
(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)

License

I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.

Summary by CodeRabbit

New Features
- Introduced drag-and-drop functionality for label attributes in the label editor.
Style
- Updated styles to support drag-and-drop handles with a pointer cursor.

klakhov

Generally im not sure if this solution is good in UX terms. Design with arrows is a bit outdated and not so comfortable to use (if I want to place 10th attribute to the first place I need to make 10 clicks 😟).
Lets return to original proposal, as the author of 7370 mentiones, it sould be implemented as drag-n-drop. I belive we need to use dots icon that is shown only on hover.
It should be something similar to:

In CVAT, we use react-grid-layout to help us implement draggable and responsible layouts on analytics page and canvas-layout. I would suggest to implement this feature using specific rows-only draggable layout.

Also, please, add a link to the issue you're trying to resolve in PR description. And dont forget to add a changelog entry.

.vscode/launch.json

KrishavRajSingh · 2024-03-18T13:04:27Z

Ok, I will make the changes

KrishavRajSingh · 2024-03-20T08:05:13Z

Generally im not sure if this solution is good in UX terms. Design with arrows is a bit outdated and not so comfortable to use (if I want to place 10th attribute to the first place I need to make 10 clicks 😟). Lets return to original proposal, as the author of 7370 mentiones, it sould be implemented as drag-n-drop. I belive we need to use dots icon that is shown only on hover. It should be something similar to: In CVAT, we use react-grid-layout to help us implement draggable and responsible layouts on analytics page and canvas-layout. I would suggest to implement this feature using specific rows-only draggable layout.

Also, please, add a link to the issue you're trying to resolve in PR description. And dont forget to add a changelog entry.

Hey I think react-grid-layout doesn't works on Forms.

klakhov · 2024-03-20T09:15:19Z

What do you mean by doesnt work on Forms?
I dont think it can work directly on the the Form.Item components. As usage examples of react-grid-layouts shows, we need to pass just a div inside of it. My first thoughts of the solution are: we can wrap Form.Item in div and then manage layout somewhere in the state of the label form component. At least antd form docs here and here show that it should be possible. Probably we need to refactor the label form component as its heavily dependent on antd form so we can manage attributes layout correctly.

Could you describe the soultion you are trying to implement so we can discuss it?

KrishavRajSingh · 2024-03-20T15:05:16Z

What do you mean by doesnt work on Forms? I dont think it can work directly on the the Form.Item components. As usage examples of react-grid-layouts shows, we need to pass just a div inside of it. My first thoughts of the solution are: we can wrap Form.Item in div and then manage layout somewhere in the state of the label form component. At least antd form docs here and here show that it should be possible. Probably we need to refactor the label form component as its heavily dependent on antd form so we can manage attributes layout correctly.

Could you describe the soultion you are trying to implement so we can discuss it?

I was trying to implement the same but after wrapping with < ReactGridLayout >, this.renderAttributes was not returning anything. Most probably because fieldInstances was empty when calling from < ReactGridLayout >.
This is a gist of the changes I made <script src="https://gist.github.com/KrishavRajSingh/0b7c9ca11b75bd677413e0be0b055702.js"></script>

klakhov · 2024-03-21T11:01:13Z

I think we dont need to pass Form.List into the grid, instead we should create a grid inside of a list. As you can see, Form.List expects a function that will render a list of Form.Items. We already use this function to render a plain list, but instead of that we can firstly create a layout and then return a react grid layout component. See the example. The thing is that we need to manage layout correcly in the component to keep the order of attributes in list. Also, need to work with the styles.

KrishavRajSingh · 2024-03-23T11:07:30Z

I think we dont need to pass Form.List into the grid, instead we should create a grid inside of a list. As you can see, Form.List expects a function that will render a list of Form.Items. We already use this function to render a plain list, but instead of that we can firstly create a layout and then return a react grid layout component. See the example. The thing is that we need to manage layout correcly in the component to keep the order of attributes in list. Also, need to work with the styles.

Hey, I have almost completed it but I am facing a problem. The thing is when we change the layout, I update the attributes list and this layout updates based on this new attributes list. These were the changes that I have made: url.
Screencast from 23-03-24 04:34:48 PM IST.webm

klakhov · 2024-03-26T08:12:48Z

Hi @KrishavRajSingh , As I can see you are trying to update form after layout changes and then it re-renders. I think we can fix this problem by managing layout somewhere outside of the form. There should be some state for the layout. By default it should be empty, then on first render of the form we set it up. If user makes some changes, we set the state of the layout object. When the form is submitted (handleSubmit func) we can use the layout state to provide correct order of the attributes to the onSubmit func.

bsekachev · 2024-03-26T14:16:01Z

Also, I would recommend to look at sorting component. It may be found on projects/tasks/cloud storages pages
Drag & Drop is implemented there

And this solution will not work when attributes were already saved on the server.

klakhov · 2024-03-30T11:29:01Z

Hi @KrishavRajSingh,
I was trying to invistigate why ui-only fix works for this issue in the first place. A brief look over the CVAT server, shows that there are no ordering fields, updated date, or explicit ordering in queryset for the Attribute model. There is no ORDER BY in sql queries to DB and no sorting in django. But still the experiments show that attributes are returned from the server with implicit sorting by updated_date(even we dont really have such field on our side). That means the fix works only because of some kind of optimization on DB side(which maybe caches rows in order of updation, I dont really know). The fact that it uses some ordering is undocumented and cant be relied on.
So it seems we need to extend the solution with more reliable approach to sorting.. We need something on the server side to support the ui. Can you think about it? Maybe we need to introduce ordering field, maybe we should add updated_date to attribute and sort by it.

KrishavRajSingh · 2024-03-31T04:26:16Z

I am looking into this.

KrishavRajSingh · 2024-03-31T15:22:35Z

Contributor

Thanks for this, I was finally able to implement this drag&drop.

Drag.Drop_CVAT.mp4

…down_Icon

klakhov · 2024-05-23T13:36:47Z

Small problem, after I change the order the drag icon becomes inactive and I cant drag once again:

cvat-ui/src/components/labels-editor/label-form.tsx

klakhov · 2024-05-23T13:37:54Z

cvat/apps/engine/views.py

@@ -49,7 +49,7 @@
 from cvat.apps.engine.frame_provider import FrameProvider
 from cvat.apps.engine.media_extractors import get_mime
 from cvat.apps.engine.models import (
-    ClientFile, Job, JobType, Label, SegmentType, Task, Project, Issue, Data,
+    ClientFile, Job, JobType, Label, AttributeSpec, SegmentType, Task, Project, Issue, Data,


Remove unused import.

cvat/apps/engine/serializers.py

cvat-ui/src/components/labels-editor/styles.scss

cvat-ui/src/components/labels-editor/label-form.tsx

klakhov · 2024-05-23T13:50:56Z

Also padding after attribute list became too small. Previously we had:

klakhov · 2024-05-23T13:55:31Z

Can we disable vertical resizing too? Horizontal is disabled as I can see

klakhov · 2024-05-24T06:19:46Z

Also please check failing linters and tests

coderabbitai

Actionable comments posted: 1

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7d6dba5 and dd35fea.

Files selected for processing (2)

cvat-ui/src/components/labels-editor/label-form.tsx (7 hunks)
cvat-ui/src/components/labels-editor/styles.scss (1 hunks)

Files skipped from review as they are similar to previous changes (1)

cvat-ui/src/components/labels-editor/styles.scss

Additional Context Used

Biome (20)

cvat-ui/src/components/labels-editor/label-form.tsx (20)

75-75: Unexpected any. Specify a different type.

157-157: Unexpected any. Specify a different type.

163-163: Unexpected any. Specify a different type.

163-163: Unexpected any. Specify a different type.

166-166: Unexpected any. Specify a different type.

166-166: Unexpected any. Specify a different type.

182-182: Unexpected any. Specify a different type.

196-196: Unexpected any. Specify a different type.

196-196: Unexpected any. Specify a different type.

241-241: Unexpected any. Specify a different type.

241-241: Unexpected any. Specify a different type.

246-246: Unexpected any. Specify a different type.

323-323: Unexpected any. Specify a different type.

345-345: Unexpected any. Specify a different type.

345-345: Unexpected any. Specify a different type.

349-349: Unexpected any. Specify a different type.

398-398: Unexpected any. Specify a different type.

408-408: Unexpected any. Specify a different type.

408-408: Unexpected any. Specify a different type.

426-426: Unexpected any. Specify a different type.

Additional comments not posted (4)

cvat-ui/src/components/labels-editor/label-form.tsx (4)

Line range hint 20-31: Import and setup of react-grid-layout for drag-and-drop functionality.

This setup is crucial for the drag-and-drop feature and has been correctly implemented. The WidthProvider HOC is used to manage the grid's width, which is a common practice with react-grid-layout.

465-501: Implementation of the drag-and-drop UI for attributes.

The UI components for drag-and-drop are well-structured and follow the design requirements. The use of HolderOutlined for the drag handle is consistent with the initial feedback from the review comments.

704-730: Integration of ReactGridLayout for managing attribute layout.

The integration of ReactGridLayout to manage the layout of attributes dynamically is a good use of the library. The onLayoutChange handler updates the form's layout state correctly, ensuring that the attribute order is preserved.

70-79: Handling of attribute reordering logic in the form submission.

The logic to reorder attributes based on the layout from the form is well-implemented. However, consider using a more specific type than any for the obj parameter in the loop to enhance type safety.

cvat-ui/src/components/labels-editor/label-form.tsx

KrishavRajSingh · 2024-05-27T08:18:35Z

Also please check failing linters and tests

I have fixed almost everything, but I am not able to run the tests locally. Please help me with that.
Thanks.

klakhov · 2024-05-28T15:20:56Z

@KrishavRajSingh
To run tests locally, please refer to our docs. I suppose you need the part with E2E tests and Rest API tests. Also, please look on linter errors.

KrishavRajSingh · 2024-06-03T07:06:33Z

@KrishavRajSingh To run tests locally, please refer to our docs. I suppose you need the part with E2E tests and Rest API tests. Also, please look on linter errors.

Hey, I am getting thia issue while generating cvat-sdk for tests.

sonarqubecloud · 2024-06-09T20:37:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

klakhov · 2024-06-19T07:48:09Z

Hi @KrishavRajSingh, sorry for the long response, we will try to look on the issue.

KrishavRajSingh · 2024-07-06T16:55:29Z

Hi @KrishavRajSingh, sorry for the long response, we will try to look on the issue.

Hey @klakhov , you may review this now. Sorry for taking this long.

KrishavRajSingh · 2024-07-06T16:58:23Z

Also I was informed that I need to make a similar drag and drop for Annotation labels, so should I make this here or should I make a separate PR?

bsekachev · 2024-07-08T10:43:42Z

@klakhov

Please, look at the updates

cvat-ui/src/components/labels-editor/label-form.tsx

cvat-ui/src/components/labels-editor/styles.scss

cvat-ui/src/components/labels-editor/label-form.tsx

klakhov · 2024-07-10T08:38:03Z

There is a bug with textarea:
Lets make dots icon smaller, and with less padding. Now it takes too much space. Something like
If I move attribute away from the page I see x-scrollbar, I dont think we need it:
When I click 'Add attribute' the from element is added, but when I start typing there is some strange padding appears(It seems it happens when my page width is small):

KrishavRajSingh · 2024-07-17T22:41:57Z

@klakhov you may review this.

klakhov

Hi @KrishavRajSingh,

As I see now you are trying to adjust height of elements based on having/not-having error using some unspecified constants as 8,6, 25 etc. I dont think this approach is correct. Consider this: I've incorrectly filled up a attibute name with two errors and now adjustments are not working:

In other case, what if we will try to add the LabelForm component on other page where the width of the container would be different. It will break the adjustments too.

We need to calculate height automatically based on current form component height(with errors) and base row height. We can use some constants but maybe for ratios, not for the height itself. There is a thread about this in RGL repo. Can we try to get the actual width of the components and resize its grid height based on it?

Also, the same comment as before, we need to make the drag element padding smaller, its too large now.

klakhov · 2024-07-22T08:05:36Z

cvat-ui/src/components/labels-editor/styles.scss

+
+.cvat-save-cancel-btn {
+    padding-top: 0.1rem;


The same comment as before, please dont use rem, use $grid-unit-size

klakhov · 2024-07-22T08:07:42Z

cvat-ui/src/components/labels-editor/styles.scss

+
+.anticon-holder > svg {
+    height: 3 / 2 * $grid-unit-size;


Suggested change

height: 3 / 2 * $grid-unit-size;

height: $grid-unit-size * 1.5;

klakhov · 2024-09-13T07:33:12Z

This PR has no activity for more than a month, I will close it for now. If you wish to finish the contribution feel free to re-open 😄

KrishavRajSingh · 2024-09-13T08:15:37Z

This PR has no activity for more than a month, I will close it for now. If you wish to finish the contribution feel free to re-open 😄

Sorry for this, am pretty busy for a while.
Will finish if I get time.
Thanks for waiting.♥️

KrishavRajSingh added 2 commits March 17, 2024 12:47

added swap functionality

f4e565d

allowed button click for relevant values

d429311

KrishavRajSingh requested review from azhavoro and bsekachev as code owners March 17, 2024 14:17

KrishavRajSingh changed the title ~~Added up/down icon for changing order of attributes~~ [GSoC2024] Added up/down icon for changing order of attributes Mar 17, 2024

klakhov reviewed Mar 18, 2024

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

klakhov requested review from klakhov and removed request for bsekachev and azhavoro March 18, 2024 12:57

klakhov added the ui/ux label Mar 18, 2024

This commit implements drag and drop feature

9047108

KrishavRajSingh and others added 3 commits March 31, 2024 22:45

Merge branch 'develop' into feat_up/down_Icon

deac786

added changelog

0a865bb

Merge remote-tracking branch 'origin/feat_up/down_Icon' into feat_up/…

ba0373e

…down_Icon

KrishavRajSingh requested a review from nmanovic as a code owner April 1, 2024 07:00

removed unnecessary change

525cc2c

KrishavRajSingh changed the title ~~[GSoC2024] Added up/down icon for changing order of attributes~~ [GSoC2024] Implemented drag snd drop feature Apr 1, 2024

KrishavRajSingh changed the title ~~[GSoC2024] Implemented drag snd drop feature~~ [GSoC2024] Implemented drag and drop feature Apr 1, 2024

fixed draggableHandle

7e123ac

klakhov reviewed May 23, 2024

View reviewed changes

Merge branch 'cvat-ai:develop' into feat_up/down_Icon

dd35fea

coderabbitai bot reviewed May 27, 2024

View reviewed changes

cvat-ui/src/components/labels-editor/label-form.tsx Outdated Show resolved Hide resolved

Merge branch 'cvat-ai:develop' into feat_up/down_Icon

29c4300

Merge branch 'cvat-ai:develop' into feat_up/down_Icon

b0aac95

KrishavRajSingh and others added 2 commits July 5, 2024 20:46

Merge branch 'cvat-ai:develop' into feat_up/down_Icon

bed870c

updated reviewed changes

6469ca2

KrishavRajSingh requested a review from klakhov July 6, 2024 17:01

klakhov reviewed Jul 10, 2024

View reviewed changes

KrishavRajSingh and others added 2 commits July 18, 2024 04:08

Merge branch 'cvat-ai:develop' into feat_up/down_Icon

60244db

updated reviewd changes

9471290

KrishavRajSingh requested a review from klakhov July 17, 2024 22:41

klakhov reviewed Jul 22, 2024

View reviewed changes

klakhov closed this Sep 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GSoC2024] Implemented drag and drop feature #7624

[GSoC2024] Implemented drag and drop feature #7624

KrishavRajSingh commented Mar 17, 2024 •

edited by coderabbitai bot

Loading

klakhov left a comment •

edited

Loading

KrishavRajSingh commented Mar 18, 2024

KrishavRajSingh commented Mar 20, 2024

klakhov commented Mar 20, 2024 •

edited

Loading

KrishavRajSingh commented Mar 20, 2024 •

edited

Loading

klakhov commented Mar 21, 2024

KrishavRajSingh commented Mar 23, 2024 •

edited by klakhov

Loading

klakhov commented Mar 26, 2024

bsekachev commented Mar 26, 2024 •

edited

Loading

klakhov commented Mar 30, 2024

KrishavRajSingh commented Mar 31, 2024

KrishavRajSingh commented Mar 31, 2024

klakhov commented May 23, 2024

klakhov May 23, 2024

klakhov commented May 23, 2024

klakhov commented May 23, 2024

klakhov commented May 24, 2024

coderabbitai bot left a comment

KrishavRajSingh commented May 27, 2024

klakhov commented May 28, 2024

KrishavRajSingh commented Jun 3, 2024

sonarqubecloud bot commented Jun 9, 2024

klakhov commented Jun 19, 2024

KrishavRajSingh commented Jul 6, 2024

KrishavRajSingh commented Jul 6, 2024

bsekachev commented Jul 8, 2024

klakhov commented Jul 10, 2024

KrishavRajSingh commented Jul 17, 2024

klakhov left a comment

klakhov Jul 22, 2024

klakhov Jul 22, 2024

klakhov commented Sep 13, 2024

KrishavRajSingh commented Sep 13, 2024

	height: 3 / 2 * $grid-unit-size;
	height: $grid-unit-size * 1.5;

[GSoC2024] Implemented drag and drop feature #7624

[GSoC2024] Implemented drag and drop feature #7624

Conversation

KrishavRajSingh commented Mar 17, 2024 • edited by coderabbitai bot Loading

Motivation and context

Checklist

License

Summary by CodeRabbit

klakhov left a comment • edited Loading

Choose a reason for hiding this comment

KrishavRajSingh commented Mar 18, 2024

KrishavRajSingh commented Mar 20, 2024

klakhov commented Mar 20, 2024 • edited Loading

KrishavRajSingh commented Mar 20, 2024 • edited Loading

klakhov commented Mar 21, 2024

KrishavRajSingh commented Mar 23, 2024 • edited by klakhov Loading

klakhov commented Mar 26, 2024

bsekachev commented Mar 26, 2024 • edited Loading

klakhov commented Mar 30, 2024

KrishavRajSingh commented Mar 31, 2024

KrishavRajSingh commented Mar 31, 2024

klakhov commented May 23, 2024

klakhov May 23, 2024

Choose a reason for hiding this comment

klakhov commented May 23, 2024

klakhov commented May 23, 2024

klakhov commented May 24, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

KrishavRajSingh commented May 27, 2024

klakhov commented May 28, 2024

KrishavRajSingh commented Jun 3, 2024

sonarqubecloud bot commented Jun 9, 2024

Quality Gate passed

klakhov commented Jun 19, 2024

KrishavRajSingh commented Jul 6, 2024

KrishavRajSingh commented Jul 6, 2024

bsekachev commented Jul 8, 2024

klakhov commented Jul 10, 2024

KrishavRajSingh commented Jul 17, 2024

klakhov left a comment

Choose a reason for hiding this comment

klakhov Jul 22, 2024

Choose a reason for hiding this comment

klakhov Jul 22, 2024

Choose a reason for hiding this comment

klakhov commented Sep 13, 2024

KrishavRajSingh commented Sep 13, 2024

KrishavRajSingh commented Mar 17, 2024 •

edited by coderabbitai bot

Loading

klakhov left a comment •

edited

Loading

klakhov commented Mar 20, 2024 •

edited

Loading

KrishavRajSingh commented Mar 20, 2024 •

edited

Loading

KrishavRajSingh commented Mar 23, 2024 •

edited by klakhov

Loading

bsekachev commented Mar 26, 2024 •

edited

Loading