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

Hide image preview and thumbnail scrollbars #4042

Merged
merged 2 commits into from
Nov 3, 2024

Conversation

casey
Copy link
Collaborator

@casey casey commented Nov 3, 2024

In #3947, we allowed scrolling in iframes. I recently visited ordinals.com on windows, and there were hideous scrollbars on nearly every inscription.

This isn't a windows-specific issue, i.e., the scrollbars are there on macOS, they just look worse on Windows, which is why we didn't notice.

This PR does a couple things:

  • Add float: left to images in the image preview. This, for some reason, makes the images the correct size, so they don't overflow, due to some mysterious CSS behavior that I don't fully understand.
  • Add back scrolling=no to inscription thumbnails. This prevents scrollbars for appearing everywhere except the actual /inscription page, which I think is better. There's no reason to allow scrolling a thumbnail, especially if the user can just click it to scroll.

I'm still not sure that we shouldn't just re-add scrolling=no to inscriptions. Users can click the content link if they want to scroll, and the bars on windows are absolutely disgusting looking. If you made an HTML inscription, previewed it on an old version of ord, and then inscribed it on mainnet, you're now getting scrollbars where before you didn't see any.

However, this PR is a pareto improvement, unlikely removing scrollbars entirely, so I think we should merge this and if the remaining scroll bars are an issue, deal with them later.

Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

LGTM

@raphjaph
Copy link
Collaborator

raphjaph commented Nov 3, 2024

Fixes #3990

@casey casey merged commit e8cedaf into ordinals:master Nov 3, 2024
5 checks passed
@casey casey deleted the hide-scrollbars branch November 3, 2024 21:56
@gmart7t2
Copy link
Contributor

gmart7t2 commented Nov 4, 2024

@casey

This prevents scrollbars for appearing everywhere except the actual /inscription page

It doesn't. See this example: it has scrolling=no but the thumbnail still displays the scrollbar: https://without21.ordstuff.info/block/868826

Screenshot_2024-11-04_09-47-31

That ord server is running origin/master which includes #4042.

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