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.

Sign up for GitHub

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: Block movers don't appear on the playground and on the widgets page #15076

Merged
merged 1 commit into from
May 6, 2019
+56 −0
Merged

Fix: Block movers don't appear on the playground and on the widgets page #15076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/edit-widgets/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,31 @@ body.gutenberg_page_gutenberg-widgets {
height: 100%;
}
}

/**
* Animations
*/

// These keyframes should not be part of the _animations.scss mixins file.
// Because keyframe animations can't be defined as mixins properly, they are duplicated.
// Since hey are intended only for the editor, we add them here instead.
@keyframes edit-post__loading-fade-animation {
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me think this animations should probably be moved to the movers stylesheet, if it's common to all usage of the editor movers?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this animation is reused in a few places, for example for the transient state in the file block, and the saving state for the save indicator. I imagine we'll want to use this in additional places, so it's nice with a mixin to help us create consistent animations.

but it does feel weird to keep duplicating these keyframes.

As a fix to the problem, 👍 👍 from me, but CC: @kjellr if you have any ideas as to how we can better reuse this animation and avoid the duplication.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to move this animation to the Animate component?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was thinking it'd be great to consolidate all of the keyframe animations that we reuse into a single place — they're currently all over the place. The animate component might be a good place for that.

That definitely seems like something that could be handled in a separate PR though. This one seems 👍 to me.

0% {
opacity: 0.5;
}
50% {
opacity: 1;
}
100% {
opacity: 0.5;
}
}

@keyframes edit-post__fade-in-animation {
from {
opacity: 0;
}
to {
opacity: 1;
}
}
28 changes: 28 additions & 0 deletions playground/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,31 @@
height: 100%;
}
}

/**
* Animations
*/

// These keyframes should not be part of the _animations.scss mixins file.
// Because keyframe animations can't be defined as mixins properly, they are duplicated.
// Since hey are intended only for the editor, we add them here instead.
@keyframes edit-post__loading-fade-animation {
0% {
opacity: 0.5;
}
50% {
opacity: 1;
}
100% {
opacity: 0.5;
}
}

@keyframes edit-post__fade-in-animation {
from {
opacity: 0;
}
to {
opacity: 1;
}
}