-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Tiny with react ui #275
Tiny with react ui #275
Conversation
Normalize some of the single instance interface
Update single MCE inserter
Adjust inserting for heading and paragraph
Allow insertion by returning string or node
Add/tinymce single/insert
Add second quote style
Try quote with cite
Remove blockquote should remove entire blockquote
Toggle lists to line breaks instead of paragraphs
…able-false Use contenteditable false for object blocks
…ub.com/mimo84/gutenberg into tiny-with-react-ui
…nto tiny-with-react-ui
…ub.com/intronic/gutenberg into tiny-with-react-ui
</div> | ||
} | ||
<div> | ||
<InlineToolbar isOpen={ inlineOpen(collapsed) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're building the UI in React, should we have a different Inline/Block toolbar depending on the current block ? Shoud this be part of the block API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@youknowriad we split these two things to explore an idea of @annaephox.
So that inline editing options are be presented to the user directly at any range selection in the same way as medium, while leaving the block-level operations attached to the block.
We (@mimo84 and I) only just started on this project and didnt understand the blocks API requirement until this PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry. My question was ambiguous. I was asking how you each block defines each control and react to its controls. For now, you have a "global" inline toolbar that shows the controls no matter the selected block.
The same applies to the block toolbar.
blockRect={ rangeRect(topBlock) } | ||
/> | ||
</div> | ||
<TinyMCEReact content={window.content} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do the content get reflected in the state? My knowledge about Redux and State based architecture is that the state should be the source of truth and it should be rendered in the components.
I think the content prop here is never updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where it starts:
https://github.com/WordPress/gutenberg/pull/275/files#diff-29319b526b66e55eb72a1e1696deee28
- we create an app state 'store' and attach the 'actions' to it
- the actions define the state transitions (state, action) -> (state)
- we pass the state store through to the App so it can dispatch action messages to the store
- the app subscribes to any state changes to the store
Then the Turducken component coordinates the UI and Tiny:
https://github.com/WordPress/gutenberg/pull/275/files#diff-aa23597862e3bf21c5f904c13462266a
- it supplies state values as props to the *Toolbars (and the DIV outline blocks which we didnt have time to move out into its own component)
- it supplies dispatch functions as props to the Tiny component which dispatch action messages to the store when content state that is relevant to the UI changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few other points:
- redux is not tied to react (they point out Deku which I'd not seen : http://redux.js.org/docs/basics/UsageWithReact.html)
- we could use other helper libraries like react-redux, rather than redux directly, which abstract things a bit and make the code cleaner to read, or terser, but a bit more opaque to understand (requires learning that specific library)
- redux works like the ClojureScript approach to app state in things like Reagent & Om which I am more familiar with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these clarifications, I'm already aware of how Redux and React works :), my question here was regarding the content
prop. Should we make it "controlled" and "updated" when the state changes (I guess it's the same question as the controlled TinyMCE question down here)
if ( this.props.focusConfig ) { | ||
this.editor.selection.moveToBookmark( bookmark ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this code comes from the per-block
prototype. My problem here is that I was not able to have a "controlled" TinyMCE, which means I was not able to call setContent
each time the content
prop changes without loosing the cursor position.
My second problem is that I was not able to get a reliable onChange
event which is essential in a state based architecture. the TinyMCE's onChange
is not triggered on each change and even when it was called, I think the content I got which editor.getContent
was different from the real content at that moment.
Maybe you could have more expertise on these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really happy to use your code for the tiny-inside-react component, I think its really well done.
We only had 2 days to work on this prototype so I just hacked the tiny component to do the minimal changes we needed to get UI state from Tiny.
My main goal was to show how the UI and UI state can be treated separately from the content state even with the single tiny concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding a 'controlled' component
- we could try it out (can restore the selection, for example)
- this would probably mean though that tiny plugins that change the dom will fail
- it seems less intrusive and risky to drive it the unmanaged way you did:
-- using the Tiny API - adding event handlers for state updates, and calling ExecCommand to get things done in the content
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I've started by trying a "controlled" approach which is closer to the React way of doing things but I failed, that's why I've used an uncontrolled TinyMCE with an imperative API.
node: null, // current node of the selection (if collapsed) or common ancestor containing the selection) | ||
range: null, // current selection range | ||
editorRef: null // reference to the editor | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we store the blocks on the store? I think this is the most important thing to have a good Separation of Concern. Having the ability to store the editor's content by block on the store and updating the UI each time this content is changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, these two things are inter-related but different, and we have some different constraints on them:
- Document content state
- UI state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
storing the blocks in the store: depends on lots of things really
- what we define as blocks
- whether blocks exist in a hierarchy
- assuming the content state is managed in the browser dom
In this Tiny-single example we just deduce the blocks using the existing block search code that looks up from the current selection up to the element which is a direct child of the body. - This implies blocks are a linear sequence.
In the Tiny-singe example we could also look up from the current selection to the nearest enclosing 'block' type element (what ever HTML5 tags or rules those are defined to be).
- This would give a hierarchy of nested blocks following the dom structure
- if we also stored those blocks in the state it would presumably need some sort of tree structure
Thanks for the prototype @mimo84 @intronic I do think having a global contenteditable and a state based architecture is the best of two worlds (the two prototypes). But As I said in my comments, I think having the block contents in the state is the most important thing to achieve this. At its current state, this prototype have the selection and I think the current selected block on the state, but not the entire content organized by blocks which is the starting point of the Separation of Concerns. Also I couldn't figure out how to define blocks (to be able to define custom blocks). |
Hi @youknowriad This is an interesting point about the UI state and content state.
|
For the multi-instance model, the single source of truth for all the state (UI and Content) is the editor state object (JS object https://github.com/WordPress/gutenberg/blob/master/tinymce-per-block/src/renderers/block/index.js#L18-L23 ) that could be moved to a Redux store https://github.com/WordPress/gutenberg/blob/master/tinymce-per-block/src/renderers/block/index.js#L18-L23 (I've just avoided another library) but you can see here that I've something similar to a "Reducer" https://github.com/WordPress/gutenberg/blob/master/tinymce-per-block/src/renderers/block/updater.js#L236 and here are the actions https://github.com/WordPress/gutenberg/blob/master/tinymce-per-block/src/renderers/block/commands.js And here is the Data Flow diagram https://github.com/WordPress/gutenberg/blob/master/tinymce-per-block/doc/data-flow.png (the blue box is the source of truth we manipulate through time and it gets rendered with React) I think if we can achieve a similar DataFlow with the single TinyMCE approach (means moving the content to state instead of relying on the DOM), we would have the best of the two worlds. I really think the UI state is less important than the content state. |
Seems like this came together pretty fast! UI-wise it's pretty far from the UI baseline at https://github.com/WordPress/gutenberg#mockups. That's not a knock on how it looks — I like the minimalism of some of the block level controls revealing themselves on hovering or clicking the type indicator. But for the purposes of comparing the prototypes on their technical underpinnings rather than perceived UI benefits, this makes it difficult. So I'll belay commenting on omissions or divergences on the UI unless you'd like that specifically, then let me know. |
We decided to open a new PR #285 we can continue the discussion there. |
Please see PR #294 which replaces this one.
Hi all, we have a prototype going for #229.
You can see it at https://intronic.github.io/gutenberg/tinymce-single-react-ui/
What is in the demo:
What works
Block Menu
Inline style Menu
Key difference in the code
What is not completed yet
Cheers,
Mike, Maurizio & Anna
@iseulde @jasmussen @androb