Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ENH: Don't center the HTML animation rendering (Fixes #11795)#11816

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

dopplershift
Copy link
Contributor

@dopplershiftdopplershift commentedAug 6, 2018
edited
Loading

Centering the animations makes them align differently than the regular
images in the notebook. Also, align attribute is somewhat deprecated.

This adjusts some of the styling as well so that the controls are
somewhat aligned with themselves.

Closes#11795 .

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

I didn't really check the control alignment, but will take your word for it that its an improvement. But I agree that not centring the animations we create makes sense (downstream user should do so when they embed them).

@timhoffm
Copy link
Member

I've hacked the CSS changes into the first animation ofhttps://splines.readthedocs.io/en/latest/bezier.html:

grafik

The result is not quite convincing:

  • The size of the slider is significantly shorter than the image. In general, since image sizes may vary, a left-aligned slider will always look a bit out of place.
  • Themargin-left: 50px creates a pseudo-alignment of the radio buttons, which in general will not work.

IMO the best solution ist to center-align all the controls with respect to the image and left-align the bounding div. This can be achieved e.g. by<div align="center">:

grafik

That would basically be sufficient. Not a CSS guru, but as propsed inhttps://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#Attributes you may additionally replace the align attibute by CSS Grid or CSS Flexbox if you want to get rid of it.

@dopplershift
Copy link
ContributorAuthor

I'm ok with that. Honestly, I just didn't have the time or will to look up the proper CSS, so thanks for doing it for me. 😁

@timhoffm
Copy link
Member

Ok. Do you update your commit?

@dopplershift
Copy link
ContributorAuthor

I will.

@dopplershift
Copy link
ContributorAuthor

Ok, updated. I also took the opportunity to replace the "deprecated"align withmargin:auto.

@timhoffm
Copy link
Member

You added margin:auto but contrary to your last comment align is still there. Is that intentional? Also can you please use a consistent style of spacing in the style tag (minor thing).

Centering the animations makes them align differently than the regularimages in the notebook. Also, align attribute is somewhat deprecated.This changes to use inline-block with a div to put the whole animationat the left, and then center the controls within that block.
@dopplershift
Copy link
ContributorAuthor

Good catch. Fixed. Actually revealed a problem wherealign was compensating for my not-quite-sufficient CSS.

@timhoffm
Copy link
Member

Sorry to bother again. For some reason the slider is not centered with this code.

grafik

@dstansby
Copy link
Member

Superseeded by#12098 I think? Feel free to re-open if I'm wrong.

@dopplershiftdopplershift deleted the fix-11795 branchOctober 8, 2018 21:56
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@dopplershift@timhoffm@dstansby@jklymak

[8]ページ先頭

©2009-2025 Movatter.jp