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

t/1: Implemented the balloon block build #2

Merged
merged 13 commits into from
Feb 18, 2019
Merged

t/1: Implemented the balloon block build #2

merged 13 commits into from
Feb 18, 2019

Conversation

oleq
Copy link
Member

@oleq oleq commented Jan 3, 2019 •
edited by pomek
Loading

Suggested merge commit message (convention)

Feature: Implemented the balloon block build. Closes ckeditor/ckeditor5#2100. Closes ckeditor/ckeditor5#1112.

Additional information

PR with the documentation.

@oleq oleq requested a review from Reinmar January 3, 2019 09:25
@oleq oleq mentioned this pull request Jan 3, 2019
Merged
2 tasks
@oleq oleq requested a review from jodator January 8, 2019 11:53
jodator
jodator suggested changes Jan 9, 2019
View reviewed changes
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

AFAICS not typos, editor works and looks OK :)

One thing that I've found is that in the manual test the block balloon is hidden under the test description:
selection_110

@oleq
Copy link
Member Author

oleq commented Jan 9, 2019

One thing that I've found is that in the manual test the block balloon is hidden under the test description:

I wouldn't worry about that.

@oleq
Copy link
Member Author

oleq commented Jan 10, 2019

Thx @pomek for the CI.

@pomek
Copy link
Member

pomek commented Jan 10, 2019

Yeah. After applying some fixes, CI seems to work fine.

@jodator
Copy link
Contributor

jodator commented Jan 22, 2019 •
edited
Loading

I wouldn't worry about that.

OK

Reinmar
Reinmar reviewed Feb 14, 2019
View reviewed changes
theme/theme.css
@@ -0,0 +1,9 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

What's this file? I think it's not used at all.

Copy link
Member

Choose a reason for hiding this comment

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

Or it is... https://github.com/ckeditor/ckeditor5-build-balloon-block/pull/2/files#diff-61c6b4c84ae09dd9e9b7a0577466fd28R32

But why is it defined in a build and not somewhere by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because by default the block toolbar positions next to the editable. It's up to integration to configure it (editable can have border/background or not, we don't know it). I figured by default we should provide such button placement it works out of the box for all integrations. And hence this file.

@Reinmar Reinmar merged commit 4fa1ed8 into master Feb 18, 2019
@Reinmar Reinmar deleted the t/1 branch February 18, 2019 15:17
@mlewand mlewand mentioned this pull request Oct 8, 2019
Closed
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Reinmar Reinmar Reinmar left review comments

@jodator jodator jodator approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement the build Introduce ckeditor5-build-balloon-block
4 participants
@oleq @pomek @jodator @Reinmar