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 asciinema player breaking site width #634

Closed
wants to merge 3 commits into from
Closed

Fix asciinema player breaking site width #634

wants to merge 3 commits into from

Conversation

garbas
Copy link
Member

@garbas garbas commented Oct 27, 2020

Fixes #597

@samueldr I wonder if we should move this fix to #flex-flexible-spacing to apply it across all the site?


This change is Reviewable

@garbas garbas requested a review from samueldr October 27, 2020 23:47
@github-actions
Copy link
Contributor

@samueldr
Copy link
Member

samueldr commented Nov 2, 2020

(Edited the issue post as it does not fix the issue.)

@samueldr
Copy link
Member

samueldr commented Nov 2, 2020

There is also a problem with the padding here. This is not the solution. I have to look a bit more into it.

The reason there was that aspect ratio thing there was to, if I remember
right, make the whole player fit in the height of a side-ways mobile
device.

Though I think I must have forgotten to finish implementing this, as
instead it just resolved to using the default font sizes.

Rather, let's do what works fine for every other sizes: a ratio of the
device width.
@samueldr samueldr changed the title remove padding on the right when on screen xs or smaller Fix asciinema player breaking site width Nov 4, 2020
@samueldr
Copy link
Member

samueldr commented Nov 4, 2020

I have reverted your changes, and applied a change that can be tested and checked.

Though, I would suggest we rebase and drop the now useless first two commits (outide of the GitHub UI!).

I simply didn't do it because rebasing on a shared branch is rude :).

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2020

@garbas garbas closed this in #636 Nov 4, 2020
@garbas
Copy link
Member Author

garbas commented Nov 4, 2020

@samueldr feel free to rebase next time :)

I'm going to open a new PR with your commit.

@garbas
Copy link
Member Author

garbas commented Nov 4, 2020

#637 is where this PR continues

@samueldr
Copy link
Member

samueldr commented Nov 4, 2020

Oops, forgot that I added a "closes" to the other PR lol.

And noted for rebasing work.

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.

Ascinema on mobile 2 (overflow)
2 participants