Opened 15 years ago
Closed 14 years ago
#4606 closed New Feature (fixed)
AutoGrow plugin
Reported by: | Garry Yao | Owned by: | Sa'ar Zac Elias |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 3.4 |
Component: | General | Version: | |
Keywords: | Confirmed Review+ | Cc: |
Description
Port AutoGrow plugin to v3.
Attachments (4)
Change History (17)
Changed 15 years ago by
comment:1 Changed 15 years ago by
I have attached my swipe at implementing the autogrow plugin. This currently works for me and I have taken the time to make it meet the CKE coding standard but I do have questions.
These is a getResizeable function exposed by some themes that is used by the resizeable plugin that comes with CKE, and there is a setHeight function exposed by the editor itself that is used in some plugins. Should I use either of those functions as opposed to the method that I use in my code? If so, which one?
Anything else stand out to anyone on the CKE team?
Thanks, Wesley Walser
comment:2 Changed 15 years ago by
Keywords: | HasPatch added |
---|
comment:3 Changed 15 years ago by
These are my personal opinions:
I don't see a editor.setHeight function, only editor.resize(width, height, isContentHeight, resizeInner) provided by the default theme. In its current status it doesn't provide any extra benefit over directly setting the height if I'm right, but you should fire also the 'resize' event. The question is what would happen if someone creates a very different theme
I think that the default value for CKEDITOR.config.autogrowMaxHeight should be left empty as in the previous plugin.
Besides those points, another question is the correct location for this plugin. I guess that it should be placed like the rest in the plugins folder, but not included in the default 'plugins' config entry, and a sample file should be provided to show how it works.
comment:4 Changed 15 years ago by
Milestone: | CKEditor 3.x → CKEditor 3.3 |
---|
comment:5 follow-up: 6 Changed 15 years ago by
Thanks Gary,
I will add the resize event and the change to the default MaxHeight soon and submit another patch.
Thanks for the feedback.
comment:6 Changed 15 years ago by
Replying to waw325:
Thanks Garry,
I will add the resize event and the change to the default MaxHeight soon and submit another patch.
Thanks for the feedback.
Oops, thanks to alfonsoml. I just assume Garry was the one commenting.
comment:7 Changed 15 years ago by
Milestone: | CKEditor 3.3 → CKEditor 3.4 |
---|
Changed 14 years ago by
Attachment: | 4606.patch added |
---|
comment:8 Changed 14 years ago by
Keywords: | Review? added; HasPatch removed |
---|---|
Owner: | set to Sa'ar Zac Elias |
Status: | new → assigned |
The patch adds the new plugin with a sample, targeting Alfonso's suggestions.
comment:9 Changed 14 years ago by
Keywords: | Review- added; Review? removed |
---|
- We're trying to have configuration options a "documentation only", avoiding initializing them in the code (as you're doing at lines 59 and 69).
- It's nice to have a dedicated sample for this one, but it needs to be listed in the samples/index.html file also.
Let's go ahead with the 100ms delay thing and see the users feedback.
Changed 14 years ago by
Attachment: | 4606_2.patch added |
---|
comment:10 Changed 14 years ago by
Keywords: | Review? added; Review- removed |
---|
There's no reason to have the same check for the max value, as it should be zero by default.
Changed 14 years ago by
Attachment: | 4606_3.patch added |
---|
comment:11 Changed 14 years ago by
4606_2.patch has some leftovers in it, please review 4606_3 instead.
comment:12 Changed 14 years ago by
Keywords: | Review+ added; Review? removed |
---|
Great job! Please commit this one in the 3.4.x branch.
autogrow plugin