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
Contributor

@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 Oct 27, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Oct 27, 2020

@samueldr
Copy link
Member

@samueldr samueldr commented Nov 2, 2020

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

@samueldr
Copy link
Member

@samueldr 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 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

@github-actions github-actions bot commented Nov 4, 2020

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

@garbas 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
Contributor Author

@garbas garbas commented Nov 4, 2020

#637 is where this PR continues

@samueldr
Copy link
Member

@samueldr 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
Linked issues

Successfully merging this pull request may close these issues.

2 participants