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

Undoing changes will scroll us back to the changed area in specific case #10639

Merged
merged 11 commits into from Jan 5, 2024

Conversation

wszymanski
Copy link
Contributor

@wszymanski wszymanski commented Dec 4, 2023

Context

As the title says. For now, it meets the requirements only from the case described in the comment.

  1. I edit a cell and press enter
  2. I scroll down the grid and click on a new cell
  3. I press ctrl z and my focus goes back to the initial cell I edited
  4. The view should auto scroll with that cell in the top of the view

Please keep in mind that it moves vertically or horizontally only when there is such a need.

How has this been tested?

Manual tests with scrolling on both axes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. https://github.com/handsontable/dev-handsontable/issues/115

Affected project(s):

  • handsontable
  • @handsontable/angular
  • @handsontable/react
  • @handsontable/vue
  • @handsontable/vue3

Checklist:

  • I have reviewed the guidelines about Contributing to Handsontable and I confirm that my code follows the code style of this project.
  • My change requires a change to the documentation.

@wszymanski wszymanski self-assigned this Dec 4, 2023
Copy link

github-actions bot commented Dec 4, 2023

Launch the local version of documentation by running:

npm run docs:review 1c1158e66159887a5c810c0bf9c0446b3dadf0c2

@wszymanski wszymanski changed the title Undoing changes does will scroll us back to the changed area Undoing changes will scroll us back to the changed area in specific case Dec 8, 2023
@wszymanski wszymanski marked this pull request as ready for review December 8, 2023 10:29
const isChangedSelection = selectedRow !== changedRow || selectedColumn !== changedColumn;

// Performing scroll only when selection has been changed right after editing a cell.
if (isInViewport === false && isChangedSelection === true) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the isChangedSelection condition.
It is in line with this comment, but does not meet the requirements of the original post of https://github.com/handsontable/dev-handsontable/issues/115 (it does not require selecting a different cell before hitting cmd+z)

Is this what we want to do with it?
cc: @evanSe

Copy link
Member

Choose a reason for hiding this comment

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

@jansiegel the current implementation in line with the comment is correct, @wszymanski we want a code sandbox for @krzysztofspilka review before this is merged to develop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jansiegel the current implementation in line with the comment is correct, @wszymanski we want a code sandbox for @krzysztofspilka review before this is merged to develop.

Here you go: https://jsfiddle.net/916z0tvc

Copy link
Member

Choose a reason for hiding this comment

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

@wszymanski we are good to merge

@wszymanski
Copy link
Contributor Author

@jansiegel I need your ✅

@wszymanski wszymanski merged commit 2e49e9b into develop Jan 5, 2024
21 checks passed
@wszymanski wszymanski deleted the feature/dev-issue-115 branch January 5, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants