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

EZP-29210: Fix admin UI anchor link scroll bug #1366

Merged
merged 2 commits into from Jun 22, 2018
Merged

EZP-29210: Fix admin UI anchor link scroll bug #1366

merged 2 commits into from Jun 22, 2018

Conversation

benkmugo
Copy link
Contributor

@benkmugo benkmugo commented Jun 1, 2018

  • fixed height on the width controller element causes the user to get
    'stuck' and unable to scroll back to the top when clicking on a link
    targeting an in page anchor

The bug is also noted in this Jira ticket:
https://jira.ez.no/browse/EZP-29210

- fixed height on the width controller element causes the user to get
'stuck' and unable to scroll back to the top when clicking on a link
targeting an in page anchor

The bug is also noted in this Jira ticket:
https://jira.ez.no/browse/EZP-29210
@benkmugo
Copy link
Contributor Author

benkmugo commented Jun 1, 2018

This new PR fixes the original issue and addresses the side-effect found on PR #1364 (closed)

@konradoboza konradoboza requested a review from glye June 4, 2018 06:10
@andrerom andrerom requested review from dfritschy and emodric and removed request for glye June 4, 2018 07:53
@emodric
Copy link
Collaborator

emodric commented Jun 4, 2018

I've reproduced the issue, so if this fixes it, I'm +1. I do however worry about 3rd party extensions that might use <div id="widthcontrol-handler"> in their own left menu implementations. What happens where there are two such elements on the page, one from the extension and one from pagelayout?

@konradoboza
Copy link
Member

This fix works (checked on Chrome, Firefox and Safari). However, is there any reason why web/design/admin/templates/parts/user/menu.tpl wasn't also taken into account?

Copy link
Collaborator

@dfritschy dfritschy left a comment

Choose a reason for hiding this comment

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

+1 provided the comment by @konradoboza is taken care of

@benkmugo
Copy link
Contributor Author

benkmugo commented Jun 4, 2018

@emodric the instance I was testing this on had the eZ Tags extension installed, which also has a menu include with the widthcontrol-handler in it. When there were multiple widthcontrol-handler elements in place (one from the pagelayout, one from the menu include) only the first one from the pagelayout was shown and active.
The others remained hidden/inactive. You can double-check, by adding back that element in one of the menu template includes to verify.

@konradoboza it looks like I missed that one. I'll check and update the commit shortly.

Thanks all for the quick feedback.

- removed widthcontrol-handler from user menu include template (admin)
@benkmugo
Copy link
Contributor Author

benkmugo commented Jun 4, 2018

The update has been applied to the user template as well now, as pointed out by @konradoboza above.

@konradoboza konradoboza requested a review from micszo June 5, 2018 07:00
@micszo micszo self-assigned this Jun 20, 2018
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

QA Approved on eZ Publish 5.4.11.1 with diff.

@micszo micszo removed their assignment Jun 20, 2018
@andrerom andrerom changed the title Fix admin UI anchor link scroll bug EZP-29210: Fix admin UI anchor link scroll bug Jun 20, 2018
@konradoboza konradoboza merged commit a844df6 into ezsystems:master Jun 22, 2018
andrerom pushed a commit that referenced this pull request Jun 29, 2018
* Fix admin UI anchor link scroll bug
- fixed height on the width controller element causes the user to get
'stuck' and unable to scroll back to the top when clicking on a link
targeting an in page anchor

The bug is also noted in this Jira ticket:
https://jira.ez.no/browse/EZP-29210

* Fix admin UI anchor link scroll bug
- removed widthcontrol-handler from user menu include template (admin)

(cherry picked from commit a844df6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants