Opened 12 years ago
Closed 12 years ago
#9580 closed Bug (fixed)
Fix samples to make them 100% valid for all possible configurations
Reported by: | Wiktor Walc | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.0 |
Component: | General | Version: | 4.0 Beta |
Keywords: | Cc: |
Description
1) samples/api.html
has "Execute link" and "Execute bold" buttons. The buttons should be hidden if plugins are not available.
2) apidialog.html
sample depends on the Link
dialog, but should it really land in the plugins/link
folder because of that? Maybe we need a more generic sample for dialogs.
3) 'samples/uicolor.html' sample right now needs the uiColor
plugin to display color picker. The little problem is that we have two things here:
- the skin that can be customized even without the uicolor plugin, which I believe deserves a simplified sample
- and the relatively big plugin, which provides the color picker dialog
There might be more samples to fix, I just mentioned the one that I've quickly noticed
Change History (16)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
5) samples/enterkey.html
requires the enterkey plugin (?), so should be probably moved to ckeditor-dev/plugins/enterkey/samples
folder, and marked with meta tags as "Advanced sample", just like plugins/divarea/samples/divarea.html
comment:3 Changed 12 years ago by
The enter mode feature is not an affiliation of the enterkey plugin, it is instead an integrated part of the editor content handling, e.g. it affects data loading, so this sample is not subjected to be moved IMO.
comment:4 Changed 12 years ago by
Is it a problem to keep plugin specific demonstration inside of basic samples (uicolor and fullpage cases)? I think it make sense as these are highly related to the feature, and it safely use the "extraPlugin" config.
comment:5 Changed 12 years ago by
Owner: | set to Garry Yao |
---|---|
Status: | new → review |
Opened dev branch t/9580 for some sample pages.
comment:6 Changed 12 years ago by
Status: | review → review_failed |
---|
Wiktor has just pointed out that plugin specific samples must be moved out of core samples, as the --skip-omitted-in-build-config (-s) param (used by default in the online builder) will keep only selected plugins.
comment:7 Changed 12 years ago by
Status: | review_failed → review |
---|
Moved all plugins dependent samples out of core, so released sample will not depend on any other plugin.
comment:9 Changed 12 years ago by
Status: | review → review_failed |
---|
- Toolbar conf sample is in two different places in dev and release versions.
- There is number of coding style issues introduced (I can help in finding them).
- There's no "Append to" sample for div area. Even I don't know when which creator is used.
- Breadcrumb in UIColor plugin sample has outdated URL. Perhaps other samples moved to plugins folder too.
- Why CKEDITOR#pluginsLoaded plugin has been added? There already is editor#pluginsLoaded and it's even documented :). So I guess that this is a mistake.
comment:10 Changed 12 years ago by
Status: | review_failed → review |
---|
- Builder doesn't not allow plugin samples to stay in the "Basic Customization" group, perhaps it's a issue, for now I'd have it as "Advanced Samples"
- Let's handle them at last.
- Append to sample for divarea plugin can be missed IMO.
- Fixed
- CKEDITOR.dialog.add is a global method which looks wrong to be called inside of an instance specific event handler, but I can revert this change.
comment:11 Changed 12 years ago by
Additionally I moved two other samples (outputxhtml & outputforflash) to plugins as well, considering they actually depends on the htmlwriter plugin.
comment:12 Changed 12 years ago by
Status: | review → review_failed |
---|
- I pushed commit adding uicolor plugin sample to dev index.
- Flash sample does not work any more.
- Submit in tabindex sample sends empty form.
comment:13 Changed 12 years ago by
I pushed few commits improving coding style in samples. Most of these issues werent' introduced by this branch, but I fixed them reviewing this branch, so they are included.
comment:14 Changed 12 years ago by
Owner: | changed from Garry Yao to Piotrek Koszuliński |
---|---|
Status: | review_failed → assigned |
comment:15 Changed 12 years ago by
Status: | assigned → review |
---|
comment:16 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Squashed by Garry & masterised by me https://github.com/ckeditor/ckeditor-dev/commit/4efab72
4)
samples/fullpage.html
looks like similar case tosamples/uicolor.html
. Full page mode is supported by CKEditor, but thedocprops
is not actually required for that.