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

Feature: Added BlockToolbar styles. #175

Merged
merged 3 commits into from May 30, 2018
Merged

Feature: Added BlockToolbar styles. #175

merged 3 commits into from May 30, 2018

Conversation

oskarwrobel
Copy link
Contributor

Suggested merge commit message (convention)

Feature: Added BlockToolbar styles. Closes https://github.com/ckeditor/ckeditor5-ui/issues/391.

Part of ckeditor/ckeditor5-ui#392.

@oskarwrobel oskarwrobel requested a review from oleq May 18, 2018 16:11
@oskarwrobel oskarwrobel mentioned this pull request May 18, 2018
Merged
oleq
oleq reviewed May 22, 2018
View reviewed changes
Copy link
Member

@oleq oleq left a comment

Choose a reason for hiding this comment

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

Out of curiosity: what's the difference between this toolbar and the BalloonToolbar (in terms of styling)? Why can't we have the same styles for both? (I'm talking especially about .ck.ck-balloon-panel-block-toolbar)

@oskarwrobel
Copy link
Contributor Author

Out of curiosity: what's the difference between this toolbar and the BalloonToolbar (in terms of styling)? Why can't we have the same styles for both? (I'm talking especially about .ck.ck-balloon-panel-block-toolbar)

I think the reason was missing ck-toolbar-container BPV class and additional drop shadow. Fixed.

oleq
oleq reviewed May 23, 2018
View reviewed changes
theme/ckeditor5-ui/components/toolbar/blocktoolbar.css
--ck-block-toolbar-button-size: var(--ck-font-size-normal);
}

.ck.ck-block-toolbar-button {
Copy link
Member

Choose a reason for hiding this comment

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

Why not .ck-block-button? IIRC it's BlockButtonView https://github.com/ckeditor/ckeditor5-ui/pull/392/files#diff-25f0cf1953c3fa884ca284a6c91b1f22R24.

Copy link
Member

Choose a reason for hiding this comment

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

Also, where does the BlockButtonView live in DOM? Which element is its parent? (it could have an impact on the class name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Button is directly in a body collection. cc @dkonopka.

Copy link
Member

Choose a reason for hiding this comment

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

Then it makes sense, I think.

@oleq oleq merged commit 6ac7511 into master May 30, 2018
@oleq oleq deleted the t/ckeditor5-ui/391 branch May 30, 2018 11:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@oleq oleq oleq approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@oskarwrobel @oleq