Opened 13 years ago
Closed 13 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 13 years ago by
comment:2 Changed 13 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 13 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 13 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 13 years ago by
| Owner: | set to Garry Yao |
|---|---|
| Status: | new → review |
Opened dev branch t/9580 for some sample pages.
comment:6 Changed 13 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 13 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 13 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 13 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 13 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 13 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 13 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 13 years ago by
| Owner: | changed from Garry Yao to Piotrek Koszuliński |
|---|---|
| Status: | review_failed → assigned |
comment:15 Changed 13 years ago by
| Status: | assigned → review |
|---|
comment:16 Changed 13 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.htmllooks like similar case tosamples/uicolor.html. Full page mode is supported by CKEditor, but thedocpropsis not actually required for that.