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.

Sign up for GitHub

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

Jump to bottom

Allow to enter data into any search field after dragging #3272

Merged
merged 3 commits into from Mar 31, 2023
Merged

Allow to enter data into any search field after dragging #3272

merged 3 commits into from Mar 31, 2023

Conversation

CarolineDenis
Copy link
Contributor

Fixes #3269

@CarolineDenis CarolineDenis requested review from maxpatiiuk and a team March 29, 2023 17:27
@CarolineDenis CarolineDenis mentioned this pull request Mar 29, 2023
Closed
maxpatiiuk
maxpatiiuk approved these changes Mar 29, 2023
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

To test:
Drag and drop should no longer start when hovering over an input field or a button
When starting drag when hovering over other elements in a line, query line should not get duplicated or stuck

chanulee1
chanulee1 suggested changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@chanulee1 chanulee1 left a comment •
edited

Choose a reason for hiding this comment

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

Able to drag query line when hovering over a button and input field.

6Ur4yfBNr6.mp4
YHHVtyJjKa.mp4

https://coldfish-issue-3269.test.specifysystems.org/specify/query/128/

@maxpatiiuk
Copy link
Member

Drag and drop when hovering over list boxes was allowed because it didn't cause bugs (in our testing)
Drag and drop for buttons should have been disabled - @CarolineDenis take a look at why it's not fixed

@realVinayak
Copy link
Collaborator

I can still reproduce it
image

@CarolineDenis
Copy link
Contributor Author

I can still reproduce it
image

I think this was fixed in the new commit

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Mar 30, 2023 •
edited

I can still reproduce it

@realVinayak don't forget to reject the commit in such cases. That then gives owners of the PR option to request a re-review when they fix an issue which would send you a notification

@realVinayak
Copy link
Collaborator

Not sure if this should be fixed in the PR or not but when using drag query fields in a dialog, the fields are behind the dialog.
Screenshot (66)

@realVinayak
Copy link
Collaborator

Also, sometimes, when using drag in the dialog, the field lines sometimes jump outside of the dialog. Like this:
image
I'm aware that is the edge, but that is irrelevant because you are just canceling the events in the PR, so this will still be reproducible. I haven't found a conclusive way to reproduce this. @CarolineDenis can you go through the code and see why this might happen? Also, this remains on the screen even if go to a different page
image
image
image

@maxpatiiuk maxpatiiuk mentioned this pull request Mar 31, 2023
Closed
@maxpatiiuk
Copy link
Member

maxpatiiuk commented Mar 31, 2023 •
edited

Out of scope for this release as we don't have time and the fix is not trivial
Opened a separate bug - https://github.com/specify/specify7/issues/new

@realVinayak
Copy link
Collaborator

Extensively tested this, works for me. @chanulee1 can you do another round and see if you catch anything weird related to this PR?

@realVinayak realVinayak self-requested a review March 31, 2023 15:35
chanulee1
chanulee1 approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@chanulee1 chanulee1 left a comment

Choose a reason for hiding this comment

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

Did not find anything weird. Cannot drag query lines by buttons or inputs.

@maxpatiiuk maxpatiiuk merged commit 7ca0f27 into production Mar 31, 2023
8 checks passed
@maxpatiiuk maxpatiiuk deleted the issue-3269 branch March 31, 2023 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxpatiiuk maxpatiiuk maxpatiiuk approved these changes

@realVinayak realVinayak realVinayak approved these changes

@chanulee1 chanulee1 chanulee1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

After moving an element in a query, you cannot enter data into any search field
4 participants
@CarolineDenis @maxpatiiuk @realVinayak @chanulee1