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 css styles for the cheat sheet so that scrolling is working #90

Closed
wants to merge 1 commit into from
Closed

fix css styles for the cheat sheet so that scrolling is working #90

wants to merge 1 commit into from

Conversation

advorkina
Copy link

@advorkina advorkina commented Jun 29, 2018

Bug: #89

@wittlock
Copy link
Collaborator

wittlock commented Jul 7, 2018

Thank you for submitting a pull request, that's always appreciated. Sorry about the slow response, struggling to find the time to spend on the computer during the summer.

I checked this out and it looks good. It does seem to always create a scrollbar, even when it's not needed, which I'm not a fan of. I tried a few quick tweaks to see if I could find an easy fix but I could get anything in place that also made it work when the scrollbar was needed.

I can take a closer look at it, but that could take me a few, to a couple of, weeks to get around to. If you'd like to take a stab at it I'll try to find time sooner to just look at it again.

@advorkina
Copy link
Author

Hi!

Thanks for checking it out. Actually it's not showing the scrollbar when not needed:
image

Or maybe can you explain me how did you test it so it was not working?

@wittlock
Copy link
Collaborator

Sorry for not getting back to you earlier. It's been a busy summer and it'll keep being busy for another couple of weeks I'm afraid. I only had a chance to very quickly test this out and I did get a scrollbar on the cheatsheet. But since you're not seeing it I feel like I should test it out some more to try and figure out what's going on so I can give a more reliable use case where it happens. Unfortunately this is unlike to happen before second half of august. I haven't forgotten about this PR though, so don't worry. :)

@wittlock
Copy link
Collaborator

wittlock commented Nov 8, 2018

Yikes, crazy fall. Finally got a chance to get back to checking this out. So it seems like I'm getting a page scrollbar with this fix, even the cheatsheet hidden (as long as it's included in the page). I'm not getting the scroll for the hotkeys unless needed, like you say:
image

Which means if the scroll is needed in the cheatsheet I get double scrolls:
image

I'm getting this in both Chrome and Firefox with a pretty minimal angular app.

I seem to be able to get rid of it by changing the following css to this:

.cfp-hotkeys {
  height: calc(100% - 80px);
}

similar to like on .cfp-hotkeys-table, but that doesn't quite feel like the proper solution.

It seems odd that you're not seeing it as well though, so I'll fiddle around some more and try a clean slate app as a common use case we can explore together, but maybe these clues will help you figure something out. I'm hoping to be able to put some more time into this project the next few weeks, but then I'll be gone most of December.

vertical-align: middle;
}

.cfp-hotkeys-table{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for consistency's sake I'd love to see a space before the bracket, like this:

Suggested change
.cfp-hotkeys-table{
.cfp-hotkeys-table {

@advorkina advorkina closed this Mar 6, 2019
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.

2 participants