Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/144: Migrated the package from SASS to PostCSS #345

Merged
merged 11 commits into from
Nov 30, 2017
Merged

T/144: Migrated the package from SASS to PostCSS #345

merged 11 commits into from
Nov 30, 2017

Conversation

oleq
Copy link
Member

@oleq oleq commented Nov 22, 2017

Suggested merge commit message (convention)

Other: Migrated the package from SASS to PostCSS to bring theme support and avoid duplicates. Closes ckeditor/ckeditor5#5304. Closes ckeditor/ckeditor5#420.

BREAKING CHANGE: The styles are no longer developed in SASS which means the .scss files became unavailable. Please refer to the Theme Customization guide to learn more about migration to PostCSS.


Additional information

Lark:

Other PRs:

.ck-tooltip__text::after {
position: absolute;

/*Without this, hovering the tooltip could keep it visible.*/
Copy link
Contributor

@dkonopka dkonopka Nov 24, 2017

Choose a reason for hiding this comment

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

Missing spaces in this and other comments here.

content: "";
width: 0;
height: 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using a nesting here?

.ck-tooltip__text {
	display: inline-block;

	&::after {
		content: "";
		width: 0;
		height: 0;
	}
}

width: auto;
height: auto;
position: static;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

-moz-user-select: none;
-webkit-user-select: none;
-ms-user-select: none;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing default user-select: none:

@define-mixin ck-unselectable {
	-webkit-user-select: none;
	-moz-user-select: none;
	-ms-user-select: none;
	user-select: none
}

Since Chrome 60 is using user-select not -webkit-user-select: none.

.ck-editor {
& .ck-sticky-panel {
& .ck-sticky-panel__content_sticky {
z-index: var(--ck-z-modal); /*#315*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing spaces in the 🗒 .


&.ck-balloon-panel_with-arrow {
&:before,
&:after {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use a double colon for pseudo-elements instead of one.

pointer-events: none;

/*This is to get rid of flickering when transitioning opacity in Chrome.
It's weird but it works.*/
Copy link
Contributor

Choose a reason for hiding this comment

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

😄


position: absolute;
top: 50%;
transform: translate( 0, -50% );
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we should consider using translate3d -> Icons looks fuzzy on Firefox.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll review, migrate to PostCSS and merge #341 after this PR.

}

.ck-dropdown__panel-visible {
display: inline-block;
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding will-change: transformis helping -> Icons looks fuzzy on Firefox

Copy link
Member Author

Choose a reason for hiding this comment

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


position: absolute;
left: 0px;
transform: translateY( 100% );
Copy link
Contributor

Choose a reason for hiding this comment

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

👉 comment

Copy link
Member Author

Choose a reason for hiding this comment

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

@oleq oleq merged commit f152dfc into master Nov 30, 2017
@oleq oleq deleted the t/144 branch November 30, 2017 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants