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

Jump to bottom

T/135: Implement a creator with sticky toolbar #152

Merged
merged 19 commits into from
May 19, 2016
Merged

T/135: Implement a creator with sticky toolbar #152

merged 19 commits into from
May 19, 2016

Conversation

oleq
Copy link
Member

@oleq oleq commented May 9, 2016

Closes #135.

@oleq oleq added this to the 0.1.0 milestone May 9, 2016
@Reinmar
Copy link
Member

Reinmar commented May 10, 2016 •
edited
Loading

I have this:

[11:34:55] Compiling 'ckeditor.css' from 2 entry points...
[11:34:55] Plumber found unhandled error:
 Error: no mixin named ck-editor

Backtrace:
    ../ckeditor5-ui-default/theme/components/stickytoolbar.scss:4

@oleq
Copy link
Member Author

oleq commented May 10, 2016 •
edited
Loading

That's strange. WFM

t/135 dfe55c2) oleq@MBP ~/CK/ckeditor5> gulp build:clean:all
[11:58:05] Using gulpfile ~/CK/ckeditor5/gulpfile.js
[11:58:05] Starting 'build:clean:all'...
[11:58:05] Deleted file '/Users/oleq/CK/ckeditor5/build'.
[11:58:05] Finished 'build:clean:all' after 217 ms
(t/135 dfe55c2) oleq@MBP ~/CK/ckeditor5> gulp build:themes
[11:58:09] Using gulpfile ~/CK/ckeditor5/gulpfile.js
[11:58:09] Starting 'build:themes'...
[11:58:09] Starting 'build:clean:themes'...
[11:58:09] Finished 'build:clean:themes' after 7.82 ms
[11:58:09] Starting 'build:icons'...
[11:58:10] Output for amd is '/Users/oleq/CK/ckeditor5/build/amd/theme/iconmanagermodel.js'.
[11:58:10] Finished 'build:icons' after 840 ms
[11:58:10] Starting 'build:sass'...
[11:58:10] Found theme entry point '/Users/oleq/CK/ckeditor5-theme-lark/theme/theme.scss'.
[11:58:10] Found theme entry point '/Users/oleq/CK/ckeditor5-ui-default/theme/theme.scss'.
[11:58:10] Compiling 'ckeditor.css' from 2 entry points...
[11:58:10] Output for amd is '/Users/oleq/CK/ckeditor5/build/amd/theme/ckeditor.css'.
[11:58:10] Finished 'build:sass' after 64 ms
[11:58:10] Finished 'build:themes' after 917 ms
(t/135 dfe55c2) oleq@MBP ~/CK/ckeditor5>

Could it be that the order of

[11:58:10] Found theme entry point '/Users/oleq/CK/ckeditor5-theme-lark/theme/theme.scss'.
[11:58:10] Found theme entry point '/Users/oleq/CK/ckeditor5-ui-default/theme/theme.scss'.

matters? What is yours?

oleq and others added 10 commits May 10, 2016 13:11
@oleq
Fixed: Error thrown when building themes because package is compiled …
df21dff 
…earlier than the core theme and dependencies are not met.
@oleq
Moved theme entry point sorting fix to gulp–level in compileThemes.
39343c4
@oleq
Tests: Added test for builder compileThemes util to ensure the order …
4b918c1 
…of compiled theme files.
@oleq
Tests: Updated FramedEditableUIView so it is a child of FramedEditabl…
9fa1827 
…eUIIframeView and extends EditableUIView.
@oleq
Tests: Updated dependencies in LegacyCreator to include FramedEditabl…
2cc73c2 
…eUIIframe and FramedEditableUIIframeView.
@oleq
Extended documentation of FramedEditableUIIframe and FramedEditableUI…
cf57571 
…IframeView.
@oleq
Moved width/height bindings from IframeView to FramedEditableUIView.
16cb8df
@Reinmar
Merge branch 'master' into t/135
119ea0c
@oleq
Updated node-sass in package.json. Removed unused gulp-sass dependency.
4322e1a
@oleq
Updated theme-lark dependency in package.json as t/8 branch joined ck…
4918153 
…editor/ckeditor5-theme-lark#master.
@Reinmar Reinmar removed this from the 0.1.0 milestone May 17, 2016
@Reinmar
Copy link
Member

Reinmar commented May 18, 2016

OK, I think that all branches are ready to merge them. Great job!

@Reinmar
Copy link
Member

Reinmar commented May 18, 2016

I was very close to merge this PR, but I found that few classes in ckeditor/ckeditor5-ui#24 misses docs.

Reinmar
Reinmar reviewed May 18, 2016
View reviewed changes
tests/_utils/ui/editableui/framed/framededitableuiiframeview.js
this.template.attributes.class.push( 'ck-editor__editable-iframe' );
this.template.attributes.style = [
'width:',
bind.to( 'width', ( w ) => `${w}px` ),
Copy link
Member

Choose a reason for hiding this comment

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

This is bad... We need to find a better solution, which would be an object format for styles. We can't rewrite all styles every time. I'll create a followup for this.

Copy link
Member

@Reinmar Reinmar May 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Beside, it doesn't do anything with the editor – it only resizes the iframe while the container stays at 100%. But this will need to be addressed once we'll be working on real support for resizing.

Copy link
Member

Choose a reason for hiding this comment

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

As for the followup: https://github.com/ckeditor/ckeditor5-ui/issues/27

@Reinmar Reinmar merged commit e14e3f8 into master May 19, 2016
@Reinmar Reinmar deleted the t/135 branch May 19, 2016 08:18
@Reinmar Reinmar mentioned this pull request Feb 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@oleq @Reinmar