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

Fix for sticky contents link overlap issue #1067

Merged
merged 5 commits into from
Jun 8, 2017
Merged

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Jun 5, 2017

  • At foot of page was possible to get 'Contents' fixed position link to briefly overlap content below it
  • Problem was due to height of parent element including height of Contents link, then changing after it was set to position fixed
  • Fixed by defaulting Contents link to position absolute so it never adds any height to the parent

Relates to alphagov/government-frontend#367

* At foot of page was possible to get 'Contents' fixed position link to briefly overlap content below it
* Problem was due to height of parent element including height of Contents link, then changing after it was set to position fixed
* Fixed by defaulting Contents link to position absolute so it never adds any height to the parent
* CSS precedence was broken making contents link still be visible at top of page when it shouldn't have been
@andysellick andysellick requested a review from NickColley June 7, 2017 12:16
}
}
position: absolute;
bottom: 0;

&--stuck-to-parent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this block used now? Can it be removed? I can't find any references to stuck-to-parent.

@fofr
Copy link
Contributor

fofr commented Jun 7, 2017

When the JS fails on mobile it looks broken:

Old

screen shot 2017-06-07 at 17 08 50

New

screen shot 2017-06-07 at 17 07 17

Copy link
Contributor

@fofr fofr left a comment

Choose a reason for hiding this comment

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

  • Needs a fix for mobile when JS isn't on/working
  • Some unused CSS can now be removed

* Fixed styles so link appears correctly if JS disabled
* Removed unnecessary class --stuck-to-parent
@andysellick
Copy link
Contributor Author

Have removed unnecessary CSS and fixed when JS disabled.

@fofr
Copy link
Contributor

fofr commented Jun 8, 2017

Link behaves nicer now, top work 💯 👍

@NickColley NickColley merged commit 01fef6d into master Jun 8, 2017
@NickColley NickColley deleted the back-to-top-link branch June 8, 2017 12:28
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.

3 participants