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

Don’t scroll up when showing confirm modals #5371

Merged
merged 5 commits into from
Apr 12, 2021
Merged

Don’t scroll up when showing confirm modals #5371

merged 5 commits into from
Apr 12, 2021

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Apr 7, 2021

Second try, replacing #5340

URL of deployed dev instance (used for testing):

Steps to test:

  • Create a bunch of tasks
  • in task list, scroll down, click »delete«, confirmation modal should show, but page should not scroll up to top
  • same for task types, users, projects, scripts, teams
  • same for own tasks in dashboard (cancel / reset actions etc)
  • Padding for those links should look the same as before

Issues:


@fm3 fm3 self-assigned this Apr 7, 2021
Copy link
Member

@philippotto philippotto 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 assume you searched for <a href="#" and replaced all occurrences?

@fm3
Copy link
Member Author

fm3 commented Apr 8, 2021

I hadn’t done the full list (only those where scrolling up could be a problem) but I did now. There is one remaining instance, which is in a Dropdown overlay. It did not look right with the LinkButton, but there is also no scrolling up happening there with the a so I’d leave it as is. The remainig href="#" are on AsyncLinks, which also don’t have this problem.

@fm3 fm3 enabled auto-merge (squash) April 12, 2021 05:36
@fm3 fm3 merged commit 28e4980 into master Apr 12, 2021
@fm3 fm3 deleted the linkbutton branch April 12, 2021 06:34
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.

Don't scroll up on "deactivate user"
2 participants