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 viewport jump when toggling help on "Administer CiviCRM" screen #13173

Merged
merged 1 commit into from
Dec 2, 2018

Conversation

christianwach
Copy link
Member

Overview

Prevents the page jumping to the anchor when clicking the discovery tab on an item on the "Administer CiviCRM" screen. Fixes https://lab.civicrm.org/dev/core/issues/557

Before

The page jumps to the anchor when clicking a discovery tab.

After

Prevents the page jumping to the anchor when clicking a discovery tab.

@civibot
Copy link

civibot bot commented Nov 29, 2018

(Standard links)

@civibot civibot bot added the master label Nov 29, 2018
@eileenmcnaughton
Copy link
Contributor

@colemanw can you take a look at this - pretty trivial

@colemanw
Copy link
Member

@christianwach out of curiosity, why are you guarding against event.preventDefault not existing? I've never seen that pattern before. Is it an IE6 thing or something?

@christianwach
Copy link
Member Author

Is it an IE6 thing or something?

Yes, it's an IE thing - can't remember which versions it affects, but it could well be an older version. I just use that code out of habit. I can remove the check if you think it's redundant.

@colemanw
Copy link
Member

colemanw commented Dec 2, 2018

One reason I might not have ever seen it is that Civi js mostly uses jQuery whereas here you are handling a raw DOM event. So I think jQuery irons out some of those cross-browser issues for us. Since we're outside that context I think it's fine to leave it in.

@colemanw colemanw merged commit bbdb4ee into civicrm:master Dec 2, 2018
@christianwach christianwach deleted the issue-557 branch December 5, 2018 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants