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

Prevent drag when rename #2067

Merged
merged 26 commits into from
Apr 5, 2022
Merged

Prevent drag when rename #2067

merged 26 commits into from
Apr 5, 2022

Conversation

tanmoyAtb
Copy link
Contributor

@tanmoyAtb tanmoyAtb commented Apr 1, 2022

closes #2051
Preventing drag events from the textinput works better than dynamically removing refs.

@render
Copy link

render bot commented Apr 1, 2022

@render
Copy link

render bot commented Apr 1, 2022

@tanmoyAtb tanmoyAtb changed the title Prevent drag when rename 2051 Prevent drag when rename Apr 1, 2022
@render
Copy link

render bot commented Apr 1, 2022

@github-actions github-actions bot added the Type: Bug Fix Added to PRs if they are addressing a bug label Apr 1, 2022
@tanmoyAtb tanmoyAtb changed the base branch from dev to fix/tbaut-save-on-blur-edit-loader-2050 April 1, 2022 08:42
Tbaut and others added 2 commits April 1, 2022 09:44
…ChainSafe/ui-monorepo into fix/prevent-drag-when-rename-2051
Base automatically changed from fix/tbaut-save-on-blur-edit-loader-2050 to dev April 1, 2022 09:22
@tanmoyAtb tanmoyAtb marked this pull request as ready for review April 1, 2022 09:51
@tanmoyAtb tanmoyAtb requested review from FSM1, Tbaut and asnaith April 1, 2022 09:51
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Unfortunately this doesn't solve the problem I mentioned in the issue. I still can't select some text in the rename input with the mouse (that's only on FF), and the input isn't focuses automatically as it should either.

Idk the drag logic well, but I don't really understand why we'd need to make this draggable, and then cancel anything that's happening, this looks hacky to me (maybe it's not though?)

Could we add this for storage as well

@tanmoyAtb
Copy link
Contributor Author

tanmoyAtb commented Apr 1, 2022

Unfortunately this doesn't solve the problem I mentioned in the issue. I still can't select some text in the rename input with the mouse (that's only on FF), and the input isn't focuses automatically as it should either.

Idk the drag logic well, but I don't really understand why we'd need to make this draggable, and then cancel anything that's happening, this looks hacky to me (maybe it's not though?)

Could we add this for storage as well

Interesting,

I found this discussion here, says its a known issue with FF, there are also some interesting hacks - looking into them
Firefox issue and hacks

Also the draggable implementation is something I found in the React DnD repo discussions.
React DnD discussion

Looking into coming up with a more solid approach to this.

Also the storage side of DnD is old code, I'm going to create a separate issue to recheck the DnD in storage overall.

@asnaith
Copy link
Member

asnaith commented Apr 4, 2022

This change is a great UX improvement, the only thing I found was the FF issue Thibaut already mentioned.

I will check it out again when FF improvements are ready.

@tanmoyAtb
Copy link
Contributor Author

tanmoyAtb commented Apr 4, 2022

Finally found nice hacks to keep FF happy as well as get the autofocus.

  • Text in rename field selectable,
  • Rename textfield autofocuses.

Created #2071 to update storage side of DnD

Copy link
Member

@asnaith asnaith left a comment

Choose a reason for hiding this comment

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

Latest changes look great, working well in firefox now

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Awesome, I added the hack for shared folder renaming as well to get the focus. I don't really know what's going on here with the text input, like why it needs the hack, maybe the fact that it appears only after the page load..

Also I saw that the trick isn't implemented on Storage but I guess it should be part of #2071

@Tbaut Tbaut enabled auto-merge (squash) April 5, 2022 16:40
@Tbaut Tbaut merged commit 4892472 into dev Apr 5, 2022
@Tbaut Tbaut deleted the fix/prevent-drag-when-rename-2051 branch April 5, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Fix Added to PRs if they are addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the new file name when renaming can't be selected
4 participants