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.

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

Implement editor size config #636

Closed
jodator opened this issue Oct 26, 2017 · 41 comments
Closed

Implement editor size config #636

jodator opened this issue Oct 26, 2017 · 41 comments
Assignees
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@jodator
Copy link
Contributor

jodator commented Oct 26, 2017

This feature is becoming more and more popular and already has some SO answer. So I'd like to bootstrap talks about this.

This issue is rather self explanatory but it would be nice to gather requirements or any known gotchas.

Probably the only current build that it makes sense is Classic Editor.

The minimal approach is to set width and/or height of the editor.

As for the behavior - current Classic editor will grow with content so how height setting should play with this? As minimal height or should it disable auto grow?

The other requested things (that was available in CKEditor 4) were to make height of the editor comparable to replaced textarea size settings (in my opinion we should skip it for now) and make editor resizeable (I think this is also separate issue).

@fredck
Copy link
Contributor

fredck commented Oct 26, 2017

My idea about this:

  • I'm unsure whether width is needed or using always 100% of the container element is the right way for it. Maybe others can give opinions, but I would start without that.

  • To make it simple, I think that a configuration named height is the most obvious thing. If defined, that's the editor fixed height, so it stays like that no matter the content.

  • Additionally, we can provide the min-height configuration. If height is not set, it sets the minimum height of the editor, but it grows with the content.

  • Finally, that can be paired with max-height. If height is not set, it allows the editor to grow up to the defined height. It can be paired with min-height.

  • CSS units are accepted, after all CSS will be used for that.

  • We don't care about the textarea size... at least for now.

  • It stays the question whether the size is applied to the editor as a whole or to the editing area. Maybe the whole editor is a better option as it gives some better control over the predicted space of the editor inside the application UI. But we need to be sure it doesn't bring troubles (e.g. when the toolbar starts floating).

I think that all the reasonable cases could be covered by the above.

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

@fredck I was going to write more on this but for the editor setting height I've found out that to set heigth of whole editor it would be required to set heights for inner containers:

selection_013

Then editing area grows with outer container - still I'm searching if it is the right way.

I'm unsure whether width is needed or using always 100% of the container element is the right way for it. Maybe others can give opinions, but I would start without that.

I guess that this is just to add an option for developers. I'd add the height option anyway.

Also changing dynamically editor.ui.view properties (if bounded in view) make ClassicEditor change it's size.

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

In action:
screencast 2017-10-26 16_18_20

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

Ugh.. I've mischeck how height behave. Obviously I was wrong as the editable area has parent's height and thus overlaps parent by the height of the toolbar.

I think that to be consistent it would make more sense to make editable view of desired height rather then whole editor. WDYT?

@oleq
Copy link
Member

oleq commented Oct 26, 2017

AFAIR in CKEditor 4 the height of the editor includes both the toolbar and the bottom space (with element path). I'm pretty sure this is what users actually need because otherwise, setting the height of the editable is pretty easy with CSS.

Setting the height of the editor isn't because both the UI and the editable must be taken into consideration and the geometry of the UI changes e.g. as the webpage is resized. In other words, the height of the editable must be adjusted so the entire editor (with toolbars etc.) is of desired height.

OTOH... I'm not sure config is the right way to do this. It will require a lot of effort from us to maintain it because it must be viewport–resize–proof etc.

I think this is the job for display: flex and it could possibly work in IE11 too. Generally speaking, I was thinking for a while if we can use Flex in the UI and this could be the right moment to check this out. Flex would speed up building toolbars, balloons, and other components too.

Check out the fiddle I created that shows how easy it is to set up a fixed–height editor with flex.

@fredck
Copy link
Contributor

fredck commented Oct 26, 2017

Awesome @oleq!

For sure we need to go with a simple CSS option. If the way @oleq proposed is not feasible (and I hope it is), then maybe applying the height to the editable only may be the way to go.

@fredck
Copy link
Contributor

fredck commented Oct 26, 2017

And this indeed may be something to check with IE11, because later on, it could be troublesome to bring it there, if we'll ever want it.

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

As I understand this adding flex sizing to editor would not require height or width config at all so the editor sizing can be styled using CSS only.

If so adding such option to editor would add bloat to API and I'd stick to CSS-only approach (with good docs for that ;) ).

@oleq
Copy link
Member

oleq commented Oct 26, 2017

Just FYI: it works in IE11
image

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

@oleq I've also checked that and it looks like it works :)

@fredck
Copy link
Contributor

fredck commented Oct 26, 2017

CSS only approach will not work because devs will not be able to predict the class names in all cases. A config option is necessary.

@oleq
Copy link
Member

oleq commented Oct 26, 2017

Can you provide an example where CSS only approach falls short?

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

@fredck I meant that with CSS only approach you have to style main editor <div> element which has .ck-editor class.

selection_016

@jodator
Copy link
Contributor Author

jodator commented Oct 26, 2017

@fredck
Copy link
Contributor

fredck commented Oct 27, 2017

Can you provide an example where CSS only approach falls short?

Suppose that the application has several different editors inside different forms. If you want to have different heights for each of them, you would have to enclose the editor in a "known" div, just for that purpose.

But let's say that the above is doable... what is even more complicated is when you have systems that let you create and configure specific fields on forms, like Drupal, through and administration panel. Here, the editors are created and configured programmatically and CSS has no idea about them.


I love the CSS only approach and I think we could even go with it only for now. It makes a lot of sense. But then we'll wait for people to scream so we'll introduce this configuration... and I'm sure people will scream. So maybe better to anticipate the trouble.

@oleq
Copy link
Member

oleq commented Oct 27, 2017

@jodator: So what you did in your recent changes is a hybrid approach? To use flex to manage the layout of the editor and the config to simply pass the width and height to the style attribute of the main element so flex can handle the rest?

In other words: flex does the magic and we don't have to worry about UI and viewport resizing, toolbar heights and any sort of math here?

@wwalc
Copy link
Member

wwalc commented Oct 30, 2017

AFAIR in CKEditor 4 the height of the editor includes both the toolbar and the bottom space (with element path).

I'm pretty sure this is what users actually need (...)

In case of CKEditor 4 you set the height of the editing area via config.height. CKEditor 4 with height set to 500:

screen shot 2017-10-30 at 10 34 51

And it makes sense. For me the use case for setting height is that I have some idea what kind of content users will be entering (e.g. short comments, contact form), I know what font size I use so I want the user/customer to enter e.g. 4 lines of text (comments) or 8 lines (contact form) before the scrollbar is shown (indicating this way to the user that he's writing too much :P).

At the same time, I have no idea how the toolbar will render, will it span into two or three rows etc.
I want to control the height of the "editing box".

because otherwise, setting the height of the editable is pretty easy with CSS.

Nope. It's not easy, because you have to apply a style using class of some nested div element,
with a class name that looks like some internal CKEditor markup, which (I'd suspect as a developer) may change at any moment.

@oleq
Copy link
Member

oleq commented Oct 30, 2017

Nope. It's not easy, because you have to apply a style using class of some nested div element,
with a class name that looks like some internal CKEditor markup, which (I'd suspect as a developer) may change at any moment.

I disagree. CSS classes are public API of the editor. They should not change in patches (1.0.x) and developers can totally depend on them. Setting height of editable boils down to .ck-editor__editable { height: ... }.

And it makes sense. For me the use case for setting height is that I have some idea what kind of content users will be entering (e.g. short comments, contact form), I know what font size I use so I want the user/customer to enter e.g. 4 lines of text (comments) or 8 lines (contact form) before the scrollbar is shown (indicating this way to the user that he's writing too much :P).

At the same time, I have no idea how the toolbar will render, will it span into two or three rows etc.
I want to control the height of the "editing box".

So the question "what should be resized: editor or editable?" is back again. Because there's this case:

To make it simple, I think that a configuration named height is the most obvious thing. If defined, that's the editor fixed height, so it stays like that no matter the content.

and they clearly contradict each other.

TBH, I'd go with editor height resize using the config because it's tricky and leave editable resizing to CSS because it's reliable and straightforward as I pointed out.

@fredck
Copy link
Contributor

fredck commented Oct 30, 2017

For me the use case for setting height is that I have some idea what kind of content users will be entering

I think that this is just one of the use cases. It is definitely expected as an additional use case that the designer would like to set the precise height of the editor as a whole, to fit it into a specific ui (for example a limited size dialog box).

Btw, we did that in that way in CKEditor 4 just because it was the easiest option. At first we wanted to set it as the whole UI height but it was way too difficult to do it because we didn't have things like the CSS flex to help us. We ended up with the "good enough" option ;)

@wwalc
Copy link
Member

wwalc commented Oct 30, 2017

Don't get me wrong: I'm fine with any solution you develop here, but I had to show you a different POV. If you implement setting height via config that sets the whole height of the editor and leave the possibility of defining manually the CSS for .ck-editor__editable (if config.height is not set), then it would be great as well, because users will have a relatively easy way to do both things.

@jodator
Copy link
Contributor Author

jodator commented Oct 30, 2017

@oleq Yes in those commit I have hybrid approach.

You can either set styles with external stylesheet like:

.ck-editor {
	width: 700px;
	height: 500px;
}

@media (max-width: 1200px) {
	.ck-editor {
		width: 450px;
		height: 200px;
	}
}

or pass config to editor instance:

ClassicEditor
	.create( document.querySelector( '#editor' ), {
		plugins: [ Enter, Typing, Paragraph, Undo, Heading, Bold, Italic ],
		toolbar: [ 'headings', 'bold', 'italic', 'undo', 'redo' ],
		config: {
			ui: {
				width: '500px',
				height: '300px'
			}
		}
	} )

or change it dynamically:

editor.ui.view.width = '1000px';

Now out-of-the-box viewport resizing adapting is best done with CSS IMHO since the programmatic way of setting width and height on the whole editor would require to listen to viewport change events.

I think that safer is to use CSS properly and check how editor components will work with flex.

Current issues that I see with CSS-only approach are:

  • how flex will work with two-line toolbar (as for now we only have controls for one line but the feature list will grow)
  • I couldn't get min-height to work with flex/current CSS as the editable area (.ck-editor__main or .ck-editor__editable) got shrunk below parents (.ck-editor) container.

@oleq
Copy link
Member

oleq commented Oct 30, 2017

how flex will work with two-line toolbar (as for now we only have controls for one line but the feature list will grow)

It should not be a problem
kapture 2017-10-30 at 15 24 13

The toolbar part of the editor has flex growing disabled so the rest (editable) will adapt to its height.

I couldn't get min-height to work with flex/current CSS as the editable area (.ck-editor__main or .ck-editor__editable) got shrunk below parents (.ck-editor) container.

Can you show the issue in the fiddle (maybe clone this one)?

@jodator
Copy link
Contributor Author

jodator commented Oct 30, 2017

@oleq: here you go: https://jsfiddle.net/jodator/zbzok303/. The problem is when you have less text then minimum to fill either height either min-height. The max-height works well.

@oleq
Copy link
Member

oleq commented Oct 31, 2017

@jodator It boils down to flex-grow: 1 for #main. Check out this fiddle.

@jodator jodator self-assigned this Oct 31, 2017
@jodator
Copy link
Contributor Author

jodator commented Oct 31, 2017

Cool to see this working. No are we going with hybrid approach (to enable Drupal case) or CSS-only? Or we should delay this issue with more investigation on flex?

@oleq
Copy link
Member

oleq commented Oct 31, 2017

@jodator It boils down to flex-grow: 1 for #main. Check out this fiddle.

I just noticed that it's broken in IE11 because of this bug. I wonder if we can get away with that if we say that min-height does not work properly in this browser. @wwalc?

Cool to see this working. No are we going with hybrid approach (to enable Drupal case) or CSS-only? Or we should delay this issue with more investigation on flex?

Let's go with the hybrid (config for editor height, CSS for editable height) but we should wait for the research before merging.

@oleq
Copy link
Member

oleq commented Oct 31, 2017

Or maybe it's not that broken after all. Can you confirm it?

It is broken when the #editor has min-height and the content is huge:

image

@Reinmar
Copy link
Member

Reinmar commented Nov 9, 2017

I'm not able to read through all the discussions, but I'd like to leave a comment anyway.

If you want to add support for setting the size through a configuration option, please be really sure that we need it. Why?

  • Because it should work with all kinds of editors. Soon, we'll start thinking how to make it work with inline editor and in the future we may not be able to make it work with some other kinds of editors.
  • Because it means adding code.
  • Because it can be done via CSS in which you can pick between using height with overflow: hidden, min-height or max-height or combination of these. It gives you far greater power.
  • Because limiting the height of the editor is actually an antipattern because it makes editing harder.
  • Because no one said that doing this via CSS is bad.

I can see that doing this research we realised that using flex helps and will be good for the overall styling abilities and we should do that change. We should also document how to change the size of the editor. But we should be super careful when adding new config options as this has several important implications for the future.

@Reinmar Reinmar added status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Nov 9, 2017
@Reinmar
Copy link
Member

Reinmar commented Jul 18, 2018

We are not asked about this and @wwalc's answer on SO does great: https://stackoverflow.com/questions/46559354/how-to-set-the-height-of-ckeditor-5-classic-editor

So I don't think that we need to change anything.

@Reinmar Reinmar closed this as completed Jul 18, 2018
@Reinmar Reinmar added the resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). label Jul 18, 2018
@wwalc
Copy link
Member

wwalc commented Jul 19, 2018

screen shot 2018-07-19 at 16 35 41

:D

@jodator

This comment has been minimized.

@SKGGitHub

This comment has been minimized.

@daka83
Copy link

daka83 commented Sep 17, 2018

editor.ui.view.editable.editableElement.style.height = '300px';

@halfnibble
Copy link

Am I the only person who thinks this is a terribly complex way to set what was previously a very simple config option? Surely there is a more common sense way to make a height option available.

@Reinmar
Copy link
Member

Reinmar commented Jan 31, 2019

The config.height option made a lot more sense in CKEditor 3-4 era. In case of CKEditor 5 adding such option is possible but:

  • The width and height of the editor are part of its look and therefore CSS is a more adequate place to control this aspect. The same applies e.g. to the colors and sizes used in the UI. In CKEditor 4 they were controlled via the config. But in CKEditor 5 it's all controlled via CSS and you can modify those styles easily.
  • Nowadays, you need to take care of making your app responsive. If this value was set via JS you'd lose the ability to use media queries to control the layout of the editor.
  • There are many kinds of editors in CKEditor 5 and setting the height of each of them can mean a different thing. Such a config option may also not apply to some of them or may require a more complex setup.
  • Height is not the only value that you may want to set. Many people want to control min-height and max-height as well. To be consistent we'd have to introduce even more config options.

@halfnibble
Copy link

I'm thinking more about the React syntax. With components like those in Material-UI, you can pass props down to key DOM elements. For example, the TextField component takes an inputProps property that you can use to apply style or className attributes to the rendered input element.
https://material-ui.com/api/text-field/

It would be cool if the React component took an editableProps property where one could define:
editableProps={{ style: { minHeight: 200 } }} for example.

@jamespedid
Copy link

@halfnibble No you are not the only one who thinks that setting the editor height here is a little ridiculous. Calling an implementation detail as part of the public api is a little nonsensical.

@callmeberzerker
Copy link

Am I right in assuming that there is no resize-addon for CKE5 due to this very issue?

@Mgsy Mgsy mentioned this issue Dec 5, 2019
@v-a-zagoruyko

This comment was marked as off-topic.

@v-a-zagoruyko
Copy link

v-a-zagoruyko commented Jan 9, 2023

What do you think about this solution? ((React Example))

We can get current editor height like this:

resizeObserver = new ResizeObserver(
  debounce((element) => {
    this.setState({ editorHeight: element[0].target.offsetHeight });
  }, 100),
);

setInitialEditorState = (editor) => {
  this.resizeObserver.observe(editor.ui.view.editable.element);
  editor.editing.view.change((writer) => {
    ...
    writer.setStyle('resize', 'vertical', editor.editing.view.document.getRoot());
  });
};

CKEditor loses height style set by resize on blur/focus, so we must set it explicitly:

<CKEditor ... onFocus={this.handleResize} onBlur={this.handleResize} />

handleResize = (_, editor) => {
  const { editorHeight } = this.state;

  editor.editing.view.change((writer) => {
    writer.setStyle('height', `${editorHeight}px`, editor.editing.view.document.getRoot());
  });
};

@pikulinpw
Copy link

For those who've been looking for a solution to dynamically adjust the height of CKEditor 5 and add resize capabilities, I'm excited to share a new plugin I've developed: https://github.com/pikulinpw/ckeditor5-resizableheight

Key Features:

  • Dynamically adjust the height of the CKEditor instance.
  • Option to resize the editor, providing a more flexible editing environment.
  • Set specific height options or allow the plugin to determine the height based on content.
  • Seamless integration with CKEditor 5.

It's now available on npm and easy to integrate with your existing CKEditor setup. Check out the README for installation and usage instructions.

Feedback, bug reports, and contributions are welcome. Let's make our CKEditor experience even better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests