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

[desk-tool] Scroll to nearest block if on focus path change #2200

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

rexxars
Copy link
Member

@rexxars rexxars commented Dec 23, 2020

Type of change (check at least one)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or update to documentation)
  • Maintenance
  • Other, please describe:

Does this change require a documentation update? (Check one)

  • Yes
  • No

Current behavior

Scrolls to top of the element containing the first segment of the focus path.

Description

When you have a portable text field at the bottom of object and have not yet focused any element in the form, then click the "activate" handler on it, it scrolls to the top of the form. This is caused by the editor attempting to scroll to the top of the first segment in the focus path (the object). Previously we scrolled to the center, which meant we sometime scrolled past the field (such as with a large form and clicking the first field). This PR scrolls to the nearest point, which seems more correct at least.

I'm still not sure I feel this is the right solution, but I can't quite remember why we wanted to scroll in the first place.

Note for release

  • Fix bug where form would scroll to top when first focusing a child of a nested field

Checklist

  • I have read the Contributing Guidelines
  • The PR title includes a link to the relevant issue
  • The PR title is appropriately formatted: [some-package] PR title (#123)
  • The code is linted
  • The test suite is passing
  • Corresponding changes to the documentation have been made

@rexxars rexxars requested a review from bjoerge December 23, 2020 20:11
@vercel
Copy link

vercel bot commented Dec 23, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sanity-io/test-studio/gqlbkp7ce
✅ Preview: https://test-studio-git-scroll-to-nearest.sanity-io.vercel.app

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

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

I can't quite remember why we wanted to scroll in the first place.

IIRC this was added after we got the ability to deep link to content that might be edited in dialogs, to make sure we had the enclosing value visible in the background.

This seems like a good quickfix, but let's see if we can address this properly (along with other issues related to focus handling) in the coming time.

@rexxars rexxars merged commit f4b66a8 into next Jan 4, 2021
@rexxars rexxars deleted the scroll-to-nearest branch January 4, 2021 18:30
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.

None yet

2 participants