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

Features: UI #127

Closed
scofalik opened this issue Feb 8, 2016 · 36 comments
Closed

Features: UI #127

scofalik opened this issue Feb 8, 2016 · 36 comments
Labels

Comments

@scofalik
Copy link

scofalik commented Feb 8, 2016

Features concept

This is a part of Features concept. This issue discusses UI. See also #129.

UI

It has not been yet decided how exactly Feature should be linked with UI however most probable solution will be that Feature creates required UI components, bind them with Feature or Commands and registers / publishes them for other parts of code to use. Those UI components might be then used by Creators to compose appropriate UI.

@pjasiun
Copy link

pjasiun commented Feb 17, 2016

Note that we also need some simple way to handle assets.

@scofalik
Copy link
Author

Connected with #133

@Reinmar
Copy link
Member

Reinmar commented Feb 17, 2016

Note that we also need some simple way to handle assets.

Yep, that will be handled in #104. I think it will be the same mechanism as for the code – the builder gathers all the files and then the bundler does the rest. And again the focus should be on simplifying the work of the bundler.

@Reinmar
Copy link
Member

Reinmar commented Feb 29, 2016

Why features define their UI? (and how to separate features from their UI?)

It would be really inconvenient for developers integrating CKEditor with their websites if they had to define that features A, B, C need such buttons and features D, E, F such select boxes and A, C, F such dialogs, etc.

A feature package should define the editing functionality of the feature as well as the default UI that it proposes. It needs to be a plug-and-play solution for a certain problem.

On the other hand, there must be ways to:

  1. Reuse the feature's editing functionality or UI in other features.
  2. Use the feature's editing functionality without the UI (headless editor or Node.js environment).

For the first point to be possible, the modular architecture was introduced. Any bigger part of code should be defined as a separate module. For instance, if a feature has a dialog it is recommended that it's defined as a separate module(s), so other features can use them. The same must be true for the editing functionality, so we define commands, converters, scheme, etc.

The second point would be a no-brainer if features didn't have to define their UI. Since this cannot be avoided, there are couple options:

  1. The developer can use a creator which doesn't create any UI. This will create a visually headless editor, however, since features need to import UI modules their code will be part of a bundle. That's not a big deal in Node.js environment (as long as the UI modules were properly coded so they don't fail to execute) but the developer may want to avoid this situation when bundling code for the browser (because of the code size).
  2. To resolve all issues mentioned in the previous point, two things could be done:
    • Features could be defined in two parts – headless and "UIed". Unfortunately, that would be pretty tiring because every single feature would need to define two modules – e.g. basicstyles/bold.js and basicstyles/bold-ui.js and the developer the editor would need to include the right pieces when building an editor. It'd be a perfect solution in terms of the architecture, but inconvenient for the developers (aren't these things contradicting each other? :D).
    • We can also create a "headless UI library", so a UI library which does nothing. This will reduce the code size to the minimum and ensure that everything works fine in Node.js (unless features are directly operating on the DOM which should never happen). However, it's a bit hacky.

I'm a bit torn, because the separation on headless features and normal features sounds that good. I'm quite sure developers will like that. On the other hand, is it worth going that far?

@Reinmar
Copy link
Member

Reinmar commented Feb 29, 2016

As I think about types of features that we'll have we can define:

  1. UI-less ones like the enter key.
  2. UI-light ones like bold (it only has a button).
  3. UI-heavy like e.g. image which has a button, dialog, dnd integration, resizing, etc. However, note that such features will usually be split into composable sub-features anyway. There will be image base, image dnd, image resizing – all as separate features.
  4. UI-only ones like autocompletion.

I think it's quite clear that separating "headless" parts of image resizing or autocompletion would be ridiculous, as well as separation UI of the bold feature. Doable but artificial.

So I'm rather leaning towards other solutions such as headless creators and headless UI lib as a last resort. But we'll need to watch out and perhaps define some recommendations on how to write features so they are UI and DOM independent.

@jodator
Copy link

jodator commented Feb 29, 2016

As I said before I don't see from architecture point of view why features define it's UI. It's like packaging two separate things in one module.

as well as separation UI of the bold feature. Doable but artificial.

Again my point of view on that is separating feature from UI (button) is much cleaner even though it will require two modules - which in my opinion is good.

Anyway it might be a problem with naming things. As default action dev might need a "feature" bold. This feature consists bold-feature-executor (anything related to bold/unbold selection, expose state etc.) and bold-feature-default-ui which is a button with a bold b. Such separation make sense when you want to build headless editor in which you don't use whole bold "feature" but only its bold-feature-executor.

Also locking features to some UI thing might be problematic later on when one would like to develop an UI for two features in one UI component (simple example instead of button there are 2 checkboxes for bold and italic). Also some default implementations might drive development of a feature in a way that changing it's visible component might be harder later on.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

I've spotted an interesting side topic here – what should the commands exposed by a feature actually do?

Let's say we've got the old table feature. It has the table button which executes the table command which:

  1. Opens the dialog.
  2. Dialog handles its commit and inserts the table.

What do we see here? That such feature's command would be totally useless in a headless editor. That the insertion algorithm (unless exposed in the API in a different way) is absolutely not reusable and not testable.

This raises a question whether a feature like table shouldn't consists of two commands (UI agnostic insertTable and openTableDialog which the button would trigger).

As I see it now, those commands should be based on two commands, because one command would either be useless in a headless environment or we wouldn't have a command to bind to a button.

@scofalik
Copy link
Author

scofalik commented Mar 1, 2016

That's why I proposed to split actual Commands from "other things" and make Command only operate on data while "other things" would do stuff you mentioned. If any part of code would like to open table dialog, it would import table feature and used a method from it.

@jodator
Copy link

jodator commented Mar 1, 2016

Meh easy ;) I think that more future proof is a generic table feature without any UI thingy (which as @Reinmar wrote above makes things harder).
Having things separated:

  • feature with an API for adding and manipulating a tables
  • some TableDialog which bond table API with UI dialog
    is somehow more clean in my mind (given that I do not dive deeply into CKEditor5 requirements as you do).

This will made more coding but I think that will be more flexible as you would have loosely coupled code inside.

@scofalik
Copy link
Author

scofalik commented Mar 1, 2016

I agree with @jodator on this one.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

No doubt that we all agree that having a generic, UI independent feature first, and then decorating it sounds best :D. We definitely all agree about that.

I'm more asking on how to do that in our reality. Two commands or one? Two modules or one? If two modules what naming convention? I don't see a clear picture of all this yet.

@fredck
Copy link
Contributor

fredck commented Mar 1, 2016

Brainstorming... no clear understanding of what I'm saying here.

Table plugin:

  • Registers the "table" feature module which contains a headless API to execute all aspects of the feature. E.g. insert a new table, edit an existing one, etc.
  • Registers an UI feature module. E.g. "table", which defaults to "button" and when executed opens a dialog which then manipulates tables through the feature API.

Then:

  • By convention UI modules follow a specific naming (or directory). The builder can be then configured for a headless editor and such modules would be "ignored" on build.
  • UI features are initialized by the core API on demand. In a headless configured editor, no initialization happens (because in fact the UI feature modules will no exist).

@scofalik
Copy link
Author

scofalik commented Mar 1, 2016

Why not having two modules in one package? Actually table stuff will be probably more than two modules anyway. Probably more like six. As @jodator proposed, we might have a TableDialog or (TableDialogController) class that would expose some API (or Command) to open a dialog. It would do it's things - check the selection to see in what table we actually are, sets some parameters in dialog and execute the original "table command" depending on values in inputs.

It's hard to say right now how many modules will be in table package and what will be their name. Maybe it's best to look at CKE4. We have column and row inserting which will be separate commands (easily testable under node, UI independent, no need for dialogs, etc.). We will probably have TableInsertCommand, TableRemoveCommand (maybe both could be included in TableCommand, TableRowCommand, TableColumnCommand. The whole feature will probably be available as a bundle inside TableFeature feature. Then we can have "helper" classes like TableDialog.

Since we decided that commands will be used for everything we could have two commands. One just for opening dialog (probably not testable under Node.js) and other will simply get some parameters and insert table in the editor.

Unless you want to review your opinion on Commands. Then I would do dialog opening as an exposed API. I am still not sure how UI will be coupled with Commands or Features.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

Brainstorming... no clear understanding of what I'm saying here.

Hm... I need to try to adjust this idea to the existing architecture, because while the idea is of course great it is too detached. But I've been already considering something similar, so that may be feasible.

@fredck
Copy link
Contributor

fredck commented Mar 1, 2016

Since we decided that commands will be used for everything

That's a misunderstanding.

A command for me looks like a single action that executes one of the dozens of features in the editor just like an enduser would do. For example, I would go with the "table" command, which simply opens the table dialog. In a headless editor, in fact, it would have no effect.

Other things are pure API. If I want to insert a table by API, I would retrieve the table feature and execute specific methods. The same to open a dialog and manipulate its contents.

@jodator
Copy link

jodator commented Mar 1, 2016

For me TableDialog is an independent thing that uses table API.
It can be in table plugin or can be in other module. The most important thing I think is that a table feature doesn't lock on TableDialog. As @fredck describe that a table plugin can have a table feature (umbrella term for all required API commands/events/etc) and some UI features like TableDialog (umbrella term for a button in toolbar that opens a table dialog).

ps. happy refactoring :trollface:.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

Why not having two modules in one package?

No problem with that, really... up to a point when we'll need to decide whether a bold feature is made of two features as well :D. While something is clear in case of a complex features like tables, it's much more unclear when applied to other cases that I mentioned in #127 (comment).

Also, this is a question of whether we want to make a super strict rule here or just a recommendation. If a rule, then what architectural consequences could such a rule have.

@fredck
Copy link
Contributor

fredck commented Mar 1, 2016

Same here.

Bold plugin (package):

  • Registers the "bold" feature module which contains a headless API to execute all aspects of the feature.
  • Registers the "bold" UI feature, which defaults to an existing UI module (button). It's an one line call, probably.

If we want to avoid code to "register" things (having unnecessary code on headless, for example), we could use configuration instead, in package.json. In other words, the above second bullet could be moved to package.json (and the same for the previous example for table).

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

Since we decided that commands will be used for everything

That's a misunderstanding.

A command for me looks like a single action that executes one of the dozens of features in the editor just like an enduser would do. For example, I would go with the "table" command, which simply opens the table dialog. In a headless editor, in fact, it would have no effect.

Other things are pure API. If I want to insert a table by API, I would retrieve the table feature and execute specific methods. The same to open a dialog and manipulate its contents.

If we follow this rule, then commands are totally UI stuff. It means that there's no other concept that would be UI independent and which would expose logic such as "is the subpart of the feature enabled now?"

Imagine how much code you would need to write if you would like to replace the whole editor UI with your own, as @jodator mentioned. If you attempt this in the easiest possible way, so by using a headless editor, then you would need to reimplement pieces of commands in your integration with a different UI.

I can't see any reasons to make some UI agnostic commands, so I'm voting for this.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

Bold plugin (package):

  • Registers the "bold" feature module which contains a headless API to execute all aspects of the feature.
  • Registers the "bold" UI feature, which defaults to an existing UI module (button). It's an one line call, probably.

As I wrote above – I'm trying to convert this to an implementable idea :D The thing is that packages do not register anything. Packages contain modules. The user needs to know what to use. So I'm trying to figure out how the builder could help, what can be automated and what the user needs to do manually.

@fredck
Copy link
Contributor

fredck commented Mar 1, 2016

If we follow this rule, then commands are totally UI stuff.

No. It's the simplest representation of the single user action that represents the feature. For instance, the "bold" command would not "click the button" but would execute the bold feature. Just like "table" could open a dialog or insert a table straight. Still a default implementation must exist and if you want to change the behavior, you need to code.

Then what "dialog" means is a different story because a custom UI implementation should be able to replace the default dialog component with its custom one.

So, my custom table button would simply call the "table" command. Then it is up to the command implementation. By default it would open a dialog and hopefully my custom dialog. So in fact, I'm not coding anything other than my custom UI components.

@oleq
Copy link
Member

oleq commented Mar 1, 2016

Other things are pure API. If I want to insert a table by API, I would retrieve the table feature and execute specific methods. The same to open a dialog and manipulate its contents.

If we follow this rule, then commands are totally UI stuff. It means that there's no other concept that would be UI independent and which would expose logic such as "is the subpart of the feature enabled now?"

That's a good point. Commands speak about state and it really matters. Both editor features and external code might want to know whether some action is possible or not i.e. in the context of current selection.

A Table Feature could bring multiple commands like insertTableCmd, insertTableRowCmd, splitTableCellCmd etc. but I'd rather see a separate Table UI module to register insertTableWithDialogCmd, which opens a dialog and makes some UI–specific kind of insertion.

Otherwise how would we handle a case when some external code calls TableFeature.splitCell( selection ) when selection has nothing to do with tables? Or the cell is not to be split for some reason? Throw error? The API must be state(selection)–driven. So I'm for command–rich implementation.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

No. It's the simplest representation of the single user action that represents the feature.

If it's the simplest representation then why opening a dialog? Why couldn't the dialog button insert a 3x3 table? Or NxN table, because the table dialog could open a table picker panel like in Google Docs.

You make an assumption here that your table command knows how it should interact with the user. How could it know it? It's just a command :D.

Despite the fact that your table button may work in at least 3 different ways, depending on what UI you'll attach, in all cases it should check its state (selection vs scheme) in the same way. So button's action may differ, but the state is the same. That state should be materialised in a form of a generic command – insertTable. Your specific button could then:

  • create another (e.g. insertTableWithDialog) command which would inherit from that base command (inheritance),
  • create another command and bind its state to the base command (composition),
  • bind its state to the base command and execute some custom code (duck tape :D).

BTW. See what native link command does. It doesn't open any dialog (confirm()). You need to provide the url to it. That makes it perfectly reusable. That's how our commands should work as well.

@fredck
Copy link
Contributor

fredck commented Mar 1, 2016

If it's the simplest representation then why opening a dialog?

Because that's the way the command has been implemented by the developer and the user has no other way to execute that command. So, in the user point of view and for the specific implementation the user has in his hands, that's the simplest representation of that command. That's why I classified commands as user actions while you want to propose it as developer API that for some unclear reason is proposed as commands and not as simple methods.

@Reinmar
Copy link
Member

Reinmar commented Mar 1, 2016

you want to propose it as developer API that for some unclear reason is proposed as commands and not as simple methods.

Yes, commands are developers' API (what else could they be?). They should be used for most popular feature actions. And those feature actions may have an accompanying state. That's why for those actions it's worth to use commands which nicely wrap action+state.

Beside, if we'll define commands only (because no one says that those "final" commands won't exist) for the final buttons' actions, we have nothing for the headless editor users. That's a clear argument for having base commands and UI-related commands if the situation requires them. If not, if the base action is absolutely stateless, then a command isn't necessary. It's optional, because, who loses something if the developer defines it? No one.

@Reinmar
Copy link
Member

Reinmar commented Mar 2, 2016

I’m trying to understand one thing while writing the spec proposal – what part of the features we need to run in Node.js?

We were talking about feature-less editor which just servers as a collaboration server client that decorates the content. But do we need to run any part of the features as well? Do we see any use case for that? Because we need to answer – what's a more importat use case – headless editor in the browser or running editor with features in Node.js?

@jodator
Copy link

jodator commented Mar 2, 2016

running editor with features in Node.js?

This one.

@Reinmar
Copy link
Member

Reinmar commented Mar 2, 2016

But why? If that's never going to be used for real the only use case I can see is running tests, bots or whatever.

@Reinmar
Copy link
Member

Reinmar commented Mar 2, 2016

To give you more context – I've asked this question myself when trying to draw a line between the "data" and "UI" parts of features. DOM listeners and any other DOM dependencies (buttons, dialogs, etc. are a hardcore DOM dependencies; but there are softer dependencies like geolocation API, onbeforeunload, localstorage, etc.).

I see a 3 layers that a features could define – data, base DOM, UI, but that sounds like too much ;/.

@Reinmar
Copy link
Member

Reinmar commented Mar 2, 2016

Some facts

  • Fact 1: The developer will be choosing feature modules that he/she wants to enable. The builder isn't able to enable some module – the editor configuration is beyond its reach. It could only help in creating it.
  • Fact 2: We can safely assume that in more than 90% of cases the developer will want features with UI. All kind of headless cases (either a input-like editor or specialised integration with existing website) are special by definition. In many of them the developer will need to know more about the editor anyway.
  • Fact 3: Developers may want to build editors in which some features are UI-less and some are normal.
  • Fact 4: There are many types of features so we cannot say that every feature consists of two parts. Also, features will be composable (you have a base feature (e.g. image) and you can add captioned image, aligned image, resizable image) what makes it even less clear how some features will be tied to UI. However, it seems quite clear that a feature may be divided into two major parts:
    • data – data conversion (to data model and to view), data manipulation methods, and schema definition,
    • user interaction – buttons, dialogs, notifications, contextual toolbars, but also DOM event listeners and other dependencies (key bindings, click, drag, onbeforeunload listeners, local storage, geo location, etc.).

Analysis

Division

The division on the feature data layer and the feature UI layer described in fact 4 theoretically draws a clear line for how features could be implemented. However, an input-like editor (which we consider quite headless because it doesn't have the "UI chrome") would need key bindings and many DOM listeners. In Node.js though, all the DOM event listeners are unnecessary and may require jsdom to not blow up. Actually, big part of the data layer may also be unnecessary. As you can see these two cases aren't the same and we would need to break features into 3 layers (data, base-DOM, UI) in order to satisfy both of them.

There's a an interesting third case though – cross-implementation compatibility. The only way how I can see this work is that in all implementation base layer of the same set of features is always loaded. This will ensure that:

  • the data can be parsed and converted to the data model and back (converters),
  • the data model understands the data (scheme).

This case seems to be very similar to Node.js scenario. You want the editor to understand the data, but not to provide editing options for it (the user interface).

Therefore, if we split features in the following way:

  • data layer – feeds the editor with data conversion, semantics and manipulation logic, fully DOM-independent,
  • user interaction – all interaction related things including all DOM-dependent stuff.

We'll satisfy 2 out of 3 cases and we'll open doors for producing a different interaction layer for some features. This may be useful e.g. on small screen touch devices where we've got very little space so we may be forced to create special UI.

Implementation

I think that the most important conclusion can be drawn from the 2nd fact – the developer creating some kind of a headless editor will need and perhaps want to better understand the structure of feature modules and possibilities they create. This case doesn't need to be as intuitive and exposed as the case in which the developer builds a normal editor and needs to quickly choose features for it.

This means that under the usual names we should keep what most of the developers will look for, so the UI-rich features (to make it clear – UI-rich features will require their UI-less partners so the user needs to remember only about the former ones).

Where to keep the UI-less base features?

  1. In separate packages? E.g. ckeditor5-image-base?
  2. In separate directories? E.g. src/base/ or src/data/?
  3. In the same directory, but with some prefix/suffix?
  4. One of the above, plus data and UI features should extend a different classes (e.g. DataFeature/Feature and UIFeature)?

I'm against 1. because that will nearly duplicate number of packages and most often you'll need to install both anyway.

Separate directories may make some sense, but there will be some features which will operate only on the data with no UI and any DOM dependency (e.g. a feature which marks all characters over 140 limit with red background, like on Twitter). Could be ok if we wouldn't move such features to src/base as they are base for themselves. But that'd mean that we cannot statically analyse to what category such features fall unless we also go for two separate classes.

As for prefixes and suffixes – they are problematic for one reason – we should rather avoid using dashes as we don't use them in class names. I think I'd be ok with:

  • bascistyle/
    • basebold
    • bold
    • baseitalic
    • italic
  • image/
    • baseimage
    • image
    • baseimagecaption
    • imagecaption
    • ...

Note: In this case we can also have features which are only manipulating on data. But if a feature is "base" it does not make sense to prefix it.

I think that I like the prefixes more. To make static analysis possible we can introduce the UIFeature class, but I don't think that it really makes much sense – I don't see a use case for it.

The only doubt I have is whether "base" is the right name. It doesn't clearly indicate that it's the "data layer" of a feature, but the question is whether that would be accurate anyway, because maybe the base isn't only about the data? I was actually also considering the word "model", but it has the same problem – converters are more of a controllers. So perhaps "base" isn't that bad name?

@jodator
Copy link

jodator commented Mar 2, 2016

Maybe shifting:

  • basicstyle
    • basebold
    • boldui
    • baseitalic
    • italicui

@scofalik
Copy link
Author

scofalik commented Mar 3, 2016

👍 @jodator

@Reinmar
Copy link
Member

Reinmar commented Mar 3, 2016

Please remember guys about the fact that the developer will have to configure the editor by specifying which features should be loaded. It will be strange if he/she has to type basicstyle/boldui, basicstyle/italicui, etc.

@Reinmar
Copy link
Member

Reinmar commented Mar 3, 2016

Short summary from my f2f talk with Fred:

  1. Simple features (like bold) should be split into two part: model and UI:
    • basicstyle/bold – main entry – acts as a preset (see Presets #135) loading bascistyle/bold/boldmodel and basicstyle/bold/boldui
    • bascistyle/bold/boldmodel – defines schema, data manipulation and conversion logic
    • bascistyle/bold/boldui – defines the button and keystroke
  2. We'll recommend that more complex features are implemented with a finer granularity. E.g.:
    • image/image – main entry
    • image/image/imagemodel – defines only schema and data manipulation
    • image/image/imageeditingview – defines converter for the editing view (how it's rendered in the editor)
    • image/image/imagedataview – defines converter for the data view (how it's outputted by the editor)
    • image/image/imageui – defines the UI

Finer granularity of the feature model (or "data" – I'm still unsure what would be the most intuitive word) will allow changing the feature's representation in the data. This is not only useful for other developers (they may use converter priorities) but can also be useful for us if we'll want to ship two versions of a features.

E.g. we've been talking in Editor Recommendations that an image should always be wrapped with a <figure>, even if it doesn't have a caption (see ckeditor/editor-recommendations#14). I can easily imagine developers who won't like it. We can easily provide a different output for them.

@jodator
Copy link

jodator commented Mar 3, 2016

Naming schema looks nice. 👍

@Reinmar
Copy link
Member

Reinmar commented May 5, 2017

Similarly to #124 – done, working, will be documented soon.

@Reinmar Reinmar closed this as completed May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants