ckeditor4-sdk icon indicating copy to clipboard operation
ckeditor4-sdk copied to clipboard

Check if it's possible to port CKEditor SDK to Umberto

Open wwalc opened this issue 6 years ago • 32 comments

Try the cheapest possible option. Use the existing HTML code for source files instead of markdown. The "Get Sample Source Code" should work as before.

wwalc avatar Jun 19 '18 14:06 wwalc

@wwalc, @AnnaTomanek, @mlewand I've made proof of concept which ports entire SDK to ckeditor-docs. Below you can find few screenshots how it looks like. You can also build and test this version by yourself. To build locally docs with sdk examples follow those steps:

  1. checkout in ckeditor-docs to sdk-port branch
  2. checkout umberto to sdk-port branch
  3. link umberto in ckeditor-docs
    • use npm link inside Umberto folder
    • use npm link umberto inside ckeditor-docs folder
  4. Update submodules in ckeditor-docs there appear ckeditor-presets dependency
  5. Use grunt docs or grunt docs-serve command to build docs with sdk samples

How samples are build:

  1. There is folder docs/sdk/examples which is used as samples source
  2. There is build ckeditor standard-all version from ckeditor-presets submodule
  3. Result is copied to docs/sdk/examples/vendors/ckeditor
  4. Folders docs/sdk/examples/vendors and docs/sdk/examples/assets are copied to output directory
  5. html files in docs/sdk/examples are processed
    • title is read and used as title of samples (it appears in navigation tree)
    • meta sdk-samples is used as names on "get samples source code" list
    • tag with class "sdk-contents" is used as sample content embed in umberto
    • tags with attribute "data-sample" are stored additionally and used during "get samples source code" building

Some more info:

  • Get samples source code require redesign, currently is just showed that is possible to be embed in document and might be easily modified
  • Navigation is sorted by file names and structure is flat, what will be changed according to provided requirement
  • There are added preset data attributes to 3 samples (basic, standard, full) to use cdn version of CKEditor instead of locally builded one. Additionally, CKEditor version in those links is set up to currently builded version of CKEditor. Which means that cdn link should updated automatically to current version of CKEditor during new releases.

screen shot 2018-07-16 at 11 55 19 screen shot 2018-07-16 at 11 55 35 screen shot 2018-07-16 at 11 55 48

msamsel avatar Jul 16 '18 10:07 msamsel

Just a note, if you want to avoid npm link dance and have git credentials configured correctly, just install given Umberto branch directly:

npm i git+ssh://[email protected]/cksource/umberto.git#sdk-port

Generates correctly, so that's cool to see!

Couple of things visible:

  • There's an annoying flash of unstyled content at the beginning. I don't see it with API docs nor docs guides, so it does look like an exclusive SDK content issue.
  • Naturally there are some things to be restyled, such as documentation link next to the headers.
  • Some of config.contentsCss values were not retained, thus content of Accessibility Checker looks ugly.

Things to do from here:

  • Allow building from ckeditor-dev submodule - that's for development purpose. So if I start work on my-awesome-feature I make a my-awesome-feature branch in CKEditor dev repo that will be linked in sdk repo branch. That way we could build it in the same fashion as we build CKEditor previews.

All in all, that's a solid ground to refresh the SDK structure.

mlewand avatar Jul 16 '18 20:07 mlewand

Just a note that some warnings are shown when building docs:

Warning: Invalid links in ./ckeditor4/4.10.0/sdk/draganddrop.html:
	'mailto:[email protected]' - Alice
	'mailto:[email protected]' - Huckleberry Finn
	'mailto:[email protected]' - Robinson Crusoe
	'mailto:[email protected]' - Little Red Riding Hood
	'mailto:' - ' + contact.name + '
Warning: Invalid links in ./ckeditor4/4.10.0/sdk/mentions.html:
	'mailto:[email protected]' - @cflores
	'mailto:' - @dwilliams
	'mailto:[email protected]' - @mwilson
	'mailto:' - @mwilson
	'mailto:{username}@example.com' - @{username}
Warning: Invalid links in ./ckeditor4/4.10.0/sdk/pastefromexcel.html:
	'assets/pfw/CKEditor4.PFW.Balance.Sheet.xlsx' - sample Excel document
	'assets/pfw/CKEditor4.PFW.Balance.Sheet.xlsx' - "Balance Sheet and Cash Flow" document
Warning: Invalid links in ./ckeditor4/4.10.0/sdk/pastefromword.html:
	'assets/pfw/CKEditor4.PFW.Sample.Recognition_of_Achievement.docx' - sample Word document
	'assets/pfw/CKEditor4.PFW.Sample.Mixed_styles.docx' - sample Word document with more complicated structure and styling
	'assets/pfw/CKEditor4.PFW.Sample.Recognition_of_Achievement.docx' - "Recognition of Achievements" document
	'assets/pfw/CKEditor4.PFW.Sample.Mixed_styles.docx' - A document with complex structure and formatting

wwalc avatar Jul 24 '18 14:07 wwalc

  • [x] As Marek already mentioned, CSS styles are missing in many samples, e.g. in https://sdk.ckeditor.com/samples/inline.html (compare the look of massive inline editing) or in Mentions tags and emoji, Drag and Drop Integration etc.
  • [x] The snippets code is missing surrounding "boilerplate" (<html><head>....</head><body>...), make sure to not lose it.
  • [x] Editor size sample should be corrected to not use 70% of width but rather fixed value in pixels.
  • [x] The Documentation links in each sample could be corrected to point to the new documentation location, also all other links that still point to https://docs.ckeditor.com/ckeditor4/docs/#!/ could start using new documentation website linking structure.
  • [x] The code block formatting (<pre>) was lost in autogrow sample https://sdk.ckeditor.com/samples/autogrow.html
  • [x] The path to CKFinder must be double checked once deploying the new documentation with SDK samples so that the image uploading examples did not stop working. Possibly reuse CKFinder installed in https://docs.ckeditor.com/ckfinder/demo/ckfinder3/ckfinder.html
  • [x] The Easy Image example tries to communicate with http://localhost:9001/ckeditor4/latest/sdk/easyimage.php Since the SDK will be hosted on S3 the php script must be located in a different place.
  • [x] "Tips" are lost, see https://sdk.ckeditor.com/samples/pastefromword.html screen shot 2018-07-24 at 17 08 34
  • [x] The width of inputs in embedding media resources is wrong (they are too short), see https://sdk.ckeditor.com/samples/mediaembed.html screen shot 2018-07-24 at 17 10 24
  • [x] Buttons do not show up in the SDK description, see for example the basic text styles example screen shot 2018-07-24 at 17 12 21 should be: screen shot 2018-07-24 at 17 12 44
  • [x] Button styles is rendered wrong in the port of https://sdk.ckeditor.com/samples/styles.html screen shot 2018-07-24 at 17 14 38 screen shot 2018-07-24 at 17 14 45
  • [x] Submit button in http://localhost:9001/ckeditor4/latest/sdk/savetextarea.html isn;t working (savetextarea.php should be placed on our server somewhere)

wwalc avatar Jul 24 '18 16:07 wwalc

When I was trying to fix "Get Sample Code", I've found some wrong markup (content inside <h2>):

screen shot 2018-07-25 at 13 21 34

dkonopka avatar Jul 25 '18 11:07 dkonopka

"Tips" are lost, see https://sdk.ckeditor.com/samples/pastefromword.html

Those tips are clearly visible in my build version. screen shot 2018-07-26 at 16 04 48

msamsel avatar Jul 26 '18 14:07 msamsel

☝️ @msamsel, I've fixed it here: https://github.com/cksource/umberto/commit/5bd39f132e7aed118e9ab111a24750d38be688a4.

dkonopka avatar Jul 26 '18 14:07 dkonopka

@AnnaTomanek, @wwalc, @mlewand what we want to do with navigation in SDK? How do you see it?

msamsel avatar Jul 27 '18 07:07 msamsel

We should keep navigation consistent with whole documentation design. It's obvious that navigation is missing categorized groups right now (like on current SDK e.g: Editor UI, Toolbar, Saving data):

Questions:

  1. Do we need filter search (PoC introduced here)? Current SDK has 59 subpages with samples, with introduced parent categories it will be very long list (~80 elements).

  2. Do we need feature to fold/unfold tree in the navigation (ATM it's not used in the Guides)?

dkonopka avatar Jul 27 '18 08:07 dkonopka

I think we will use just a few groups (folders) to group the most obvious examples like presets or end user features, without making so granular categories as before.

Ideally, just one category should be expanded at the beginning, while the other categories should be collapsed. But this is a feature which is even more important for https://docs.ckeditor.com/ckeditor4/latest/guide/index.html So for me, to not block the SDK migration, just assume that we'd have a few "folders" there (End-user Features, Integration features, Editor presets etc.).

Regarding questions:

  1. For me the filter looks cool. But it must support searching in more than just a link title ("meta information) - I added my comment about it there.

  2. Yes, but it should not be a blocker.

wwalc avatar Jul 27 '18 08:07 wwalc

  • [x] Note: ideally, the created CKEditor 4 build should be placed and requested from a versioned folder. To avoid caching issues in demos. We have added this feature in SDK some time ago exactly because of this kind of issues.

Even although we clear CDN cache on CloudFront whenever we deploy the new documentation, I've noticed that locally I still have to reload the page to see changes on docs.ckeditor.com.

So, ideally, if the documentation is requested form the /latest/ location, the linked library in examples should be loaded from folder such as /4.10.0/.

  • [x] The editor is not built yet when I'm using the bigbang-docs repo. It only works in ckeditor-docs.

wwalc avatar Aug 01 '18 12:08 wwalc

@wwalc, @AnnaTomanek, @mlewand We can make another round of feedback. There are sdk-port branches in 3 repositories: ckeditor-docs, umberto, bigbang-docs. It's required to install dependencies in ckeditor-docs if you haven't done it earlier. It's also required to use git submodule update --init --recursive inside ckeditor-docs to install presets dependancies properly.

Things to correct:

  • php scripts
  • change folder of ckeditor used in samples (I need to talk about it with @wwalc)

Sample picture: screen shot 2018-08-02 at 12 02 50

msamsel avatar Aug 02 '18 10:08 msamsel

Bugs found so far:

  • [x] Make sure all issues raised so far were addressed (like correcting links to docs, replacing generic HTML links with Umberto/jsodc syntax where possible).
  • [x] Links in ckeditor-docs should no longer point to https://sdk.ckeditor.com/
  • [x] Lack of support for data-sample-short attribute in "Get Sample Source Code". See https://github.com/ckeditor/ckeditor-sdk/blob/fe1ab83928d8fb62142616714254877e4f4bb8e2/samples/assets/simplesample.js#L469 and https://github.com/ckeditor/ckeditor-sdk/blob/fe1ab83928d8fb62142616714254877e4f4bb8e2/samples/assets/simplesample.js#L19 - because of that generated code samples contain unnecessary long content in textarea element.
  • [x] It looks like the active item in the sidebar navigation isn't highlighted and the navigation does not scroll to the right place.
  • [x] data-sample-preservewhitespace sin't removed form the sample source code. Also if you look at the codesnippet example you will see that the white space is lost for legs: 4
  • [x] autocomplete example styling is broken

wwalc avatar Aug 02 '18 15:08 wwalc

  • [x] Remove from the SDK examples "Note: CKEditor SDK is being expanded all the time. Any bugs and tips should be reported here. Thanks!" Add next to each example screen shot 2018-08-03 at 15 41 36 instead.

wwalc avatar Aug 03 '18 13:08 wwalc

Current status:

  • [x] navigation issues will be done as follow up. If I understand the case properly this is the issue which includes this change: https://github.com/cksource/umberto/issues/570
  • [x] links in origin docs (markdown files) should be corrected to new sdk localization
  • [x] ckeditor folder should be moved next to assets to be served from versioning one
  • [x] php files should be moved to the server
  • [ ] editor size styling should be improved

Other stuff should be solved in current sdk-port branches.

msamsel avatar Aug 07 '18 14:08 msamsel

@wwalc @AnnaTomanek I've add new link syntax inside documentation to link to SDK samples. And convert links to use it. Take a look if it's ok for you: https://github.com/ckeditor/ckeditor-docs/commit/671851ad1efe594745962e760c7c7bfb4e094833

msamsel avatar Aug 10 '18 15:08 msamsel

I had a look at it, and good job so far, really like it.

Issues

  • [x] General: the active article is not reflected in the navigation tree. Open "Editor Auto Grow" example, it should have a gray background. Guides do have this feature.
  • [x] Accessibility Checker example's "Accessibility Checker" Sample Source Code listing:
    • [x] Includes <script src="https://code.jquery.com/jquery-2.2.3.min.js"></script> reference twice.
    • [x] Has a wrong formatting in contentsCss: [CKEDITOR.basePath + 'contents.css', 'assets/accessibilitychecker/contents.css'], should have spaces, and be split across lines for better readability.
  • [x] Code listing modal dialog scroll bar appears in a weird position https://i.imgur.com/3de0FJm.png.
  • [x] I'm not sure if it was intentional, or not, but the order of entries in navigation got changed. For instance Code Snippet is displayed before Paste from Word, which makes very little sense.
  • [x] I really dislike the "Documentation" link under the header… it doesn't lay well with anything else, it also has too large hover area.
  • [x] The list overlaps incorrectly in Autocomplete sample.
  • [x] The Inline Editor (sdk/inline.html) sdk sample:
    • [x] It's missing the styles.
    • [x] "Inline editing enabled by code" Sample Source Code is buggy, it has something like that:
    script type="template">
    &lt;script&gt; // The inline editor should be enabled on an element with "contenteditable" attribute set to "true". // Otherwise CKEditor will start in read-only mode. var introduction = document.getElementById( 'introduction' ); introduction.setAttribute(
    'contenteditable', true ); CKEDITOR.inline( 'introduction', { // Allow some non-standard markup that we used in the introduction. extraAllowedContent: 'a(documentation);abbr[title];code', removePlugins: 'stylescombo', extraPlugins: 'sourcedialog',
    // Show toolbar on startup (optional). startupFocus: true } ); &lt;/script&gt;
    /script>
    
  • [x] Floating User Interface: the editable (when it is not active) should be marked, slightly gray background like it is in current floatinspace sample seems to be a good option.
  • [x] Code samples are not working on IE11.
  • [x] "Editing Complete HTML Pages" - there's some stray hite margin between the toolbar and editable.

Enhancements

All of the following points could be as well extracted into separate issues, that's totally fine.

  • [ ] I'd like to see a small improvement in "Get Sample Source Code" header. It would be nice to add some sort of paper, or <code> icon next to it.
  • [ ] It would be nice to pre-define the content of issue created after clicking "Report an issue" icon. It should contain:
  • [x] Modal dialog should be gone, I believe showing code listing in dialog is an antipattern. Maybe it's better idea just to make these foldable.

mlewand avatar Aug 17 '18 12:08 mlewand

Modal dialog should be gone, I believe showing code listing in dialog is an antipattern. Maybe it's better idea just to make these foldable.

Completely agree, but I would like to hear your opinion about Download button. Do you really think it is neccessary? I can't find it in any docs, looks like Copy is enough for the users.

Proposal of accordion "Sample code"

screen shot 2018-08-21 at 14 47 20

dkonopka avatar Aug 21 '18 13:08 dkonopka

Usually there's just "copy"option in other documentations because "partial" code snippets are listed there. Here you have the source code of a full file in each case, that's why "download" link makes sense.

wwalc avatar Aug 21 '18 13:08 wwalc

@dkonopka I have one more trouble with proper styling inside inline editor. There are samples with this kind of editor like: https://sdk.ckeditor.com/samples/savetextarea.html or https://sdk.ckeditor.com/samples/floatingui.html Styles from Umberto leaks to inline editor (Editor takes styles from page). Generally there an issue with img styles Maybe you will have some idea how to improve it? Fix those samples, or maybe provide some classes in Umberto to prevent such case? What do you think about it?

Umberto:

screen shot 2018-08-23 at 14 14 32

Old SDK:

screen shot 2018-08-23 at 14 14 42

msamsel avatar Aug 23 '18 12:08 msamsel

@msamsel I've just fixed it, CSS Specificity for the main <h1> was too generic.

screen shot 2018-08-23 at 16 49 29

dkonopka avatar Aug 23 '18 14:08 dkonopka

@wwalc current stage is that I think pretty much all bugs was fixed. remain things:

  • [x] correct EasyImage js script to work with php script (the same which is used on main page here: https://ckeditor.com/ckeditor-cloud-services/easy-image/ )
  • [x] correct links with basic/full presets in get source code section.

Docs should build from sdk-port branches in ckeditor-docs, big-bang-docs and umberto.

msamsel avatar Aug 23 '18 14:08 msamsel

I have some doubts about what happened with the documentation link in the samples.

It used to be really prominent, to make it clear that the samples are complemented with docs: screen shot 2018-08-24 at 14 57 19

Now it landed as a small icon with no caption that I honestly totally ignored as "it's so small that it must be insignificant, there is this info or whatever else there, maybe it's the source or license or whatever else that I don't care about": screen shot 2018-08-24 at 14 48 45

Can we have a different proposal for this, please?

AnnaTomanek avatar Aug 24 '18 12:08 AnnaTomanek

Blockers

  • [x] It looks like the editor is not created correctly when building the documentation website. I had to check the examples by running grunt docs-serve in ckeditor-docs.
  • [x] The HTML source code in code examples inside
  • [x] Invisible link to the feature documentation mentioned above.
  • [x] https://github.com/ckeditor/ckeditor-sdk/issues/276 (it's so simple to solve the problem that was causing our headache for a long time so let's do it)
  • [x] The autotag example is broken (last tutorial) in Chrome.
  • [x] The url to examples should include "examples" not "sdk" (e.g. ckeditor4/latest/examples/timestamp.html not ckeditor4/latest/sdk/timestamp.html).
  • [x] Make sure CKEditor 4 examples include the correct tracking code for Google Analytics.

Optional

  • [ ] The extra "sdk" folder in ckeditor-docs is misleading and I guess not needed? Can we get rid of it to have just "examples"?

wwalc avatar Aug 24 '18 13:08 wwalc

The autotag example is broken (last tutorial) in Chrome.

@wwalc, @AnnaTomanek it seem like this sample is not working in current SDK as well: https://sdk.ckeditor.com/samples/autotag.html

I see there was change in repository and now plugins are distributed differently. It will be required updated of files which was used to move SDK to Umberto

msamsel avatar Aug 28 '18 09:08 msamsel

After consultation with @wwalc it seems to be issue somehow related to uBlock plugin in Chrome.

msamsel avatar Aug 28 '18 10:08 msamsel

@AnnaTomanek Because of GitHub icons location in the header, the only solution to improve discoverability of documentation link is simple: WDYT?

screen shot 2018-08-28 at 12 25 46

dkonopka avatar Aug 28 '18 10:08 dkonopka

@dkonopka Visually looks 👍 for me. As for the link text it can be corrected at any moment when @AnnaTomanek is back.

wwalc avatar Aug 28 '18 10:08 wwalc

It looks like the editor is not created correctly when building the documentation website. I had to check the examples by running grunt docs-serve in ckeditor-docs.

@wwalc I cannot reproduce it. It seems to work fine for me. I know that I correct script responsible for building to wait till the end. This should also fix mentioned situation. Maybe git made some tricks and doesn't refresh this script for you?

msamsel avatar Aug 28 '18 11:08 msamsel

It looks like the editor is not created correctly when building the documentation website. I had to check the examples by running grunt docs-serve in ckeditor-docs.

@wwalc I cannot reproduce it. It seems to work fine for me. I know that I correct script responsible for building to wait till the end. This should also fix mentioned situation. Maybe git made some tricks and doesn't refresh this script for you?

It seems that editor was build completely asynchronously without waiting for rest of the process. That's why it worked after first fail. Right now it's working as it should.

msamsel avatar Aug 29 '18 14:08 msamsel