Add: Code editor to edit site #37765
Conversation
jorgefilipecosta
added
[Type] Feature
|
Size Change: +4.26 kB (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
jorgefilipecosta
force-pushed
the
add/code-editor-to-edit-site
branch
2 times, most recently
from
15f5089
to
53b24d7
Compare
|
will close #22528 if merged, right? :) |
Exactly, I updated this PR and linked it to the issue. |
jorgefilipecosta
force-pushed
the
add/code-editor-to-edit-site
branch
from
53b24d7
to
f458c1a
Compare
jorgefilipecosta
force-pushed
the
add/code-editor-to-edit-site
branch
from
f458c1a
to
01248e2
Compare
| import { displayShortcut } from '@wordpress/keycodes'; | ||
| import { compose } from '@wordpress/compose'; | ||
| import { useDispatch, useSelect } from '@wordpress/data'; | ||
| import { CodeEditorScreen } from '@wordpress/interface'; |
There was a problem hiding this comment.
Why CodeEditorScreen is part of the interface package? It seems very misplaced for me?
There was a problem hiding this comment.
We need to share code between edit-post and edit-site. The interface package already contains things like the logic for the fullscreen mode, more menu, it seems to be the package where we have general infrastructure UI for editors. It is a private package so we are not exposing anything. What would be in your opinion the better place to put this code?
There was a problem hiding this comment.
I think sharing code just to share code is bad, that said, I think I may have misunderstood this component first, what is it for? Basically, how would you describe it and when one should use it?
There was a problem hiding this comment.
CodeEditorScreen is a skeleton for the editor code editor mode. It renders a big element where one can nest areas relevant for code editing (normally the code editor and maybe the post title) and also renders a button to exit the code editor.
I think sharing code just to share code is bad
In this case, we have a UI for editing posts that is exactly the same between post editor and site editor. This UI has lots of CSS, duplicating all of this seems bad, soon one will have to do a small design change and will only change in one place and forget to change it on the other.
| import { useInstanceId } from '@wordpress/compose'; | ||
| import { VisuallyHidden } from '@wordpress/components'; | ||
|
|
||
| export default function CodeEditor( { value, onChange, onInput } ) { |
There was a problem hiding this comment.
Should this be more a @wordpress/components component instead?
There was a problem hiding this comment.
The component does not seem very generic and the styles are a little bit specific to our needs (I think it would only be useful for someone building an editor behaving exactly like ours). But I can move some styles to CodeEditorScreen and try to make it more generic.
Another option is to not have a CodeEditor component and expose it under CodeEditorScreen something like CodeEditorScreen.Editor.
Let me know what you would prefer.
There was a problem hiding this comment.
It bothers me a bit to have "Code Editor" in "@interface" package with or without screen. Interface should be about building screens why do we need to know about code editor.
There was a problem hiding this comment.
Would it make sense to introduce a new @wordpress/code-editor package (akin to @wordpress/editor)?
There was a problem hiding this comment.
Would it make sense to introduce a new @wordpress/code-editor package (akin to @wordpress/editor)?
I think that makes sense but because I thought it'd be smallish I thought we could just move the component here to the components package.
There was a problem hiding this comment.
I think that makes sense but because I thought it'd be smallish
I agree, the package although making sense conceptually would be very small.
I thought we could just move the component here to the components package.
I can make a try to make it part of the components package but it seems very specific with our dimensions, our styles, etc.. This will not be useful to anyone outside us and having these components as part of our public API just makes it something we need to keep forever. For example, if we had not exposed PostTextEditor on wp.editor Probably right now I would not divide between CodeEditorScreen and CodeEditor and would just have a single component. But as we need to keep PostTextEditor I'm having a CodeEditor component so PostTextEditor can just use it. I think adding more components to wp.components will just be something more we have to maintain in the future.
Ideally, we just want to share the code between edit-post and edit-site. The component should look mostly equal in both places and I think repeating hundreds of lines of code that we need to change in two places each time the UI changes is not a good idea. The interface package was a simple solution it is private can be used by edit-post and edit site.
We may consider creating another private package to put artifacts reused between editor, edit-post, and edit-site that we don't think are something worth exposing (maybe named editor-utils?). The screens are mostly equal and we should make it easy to share artifacts between them without having to think much.
If we don't create a new package:
If we add CodeEditor to wp.components can we keep CodeEditorScreen on wp.interface? Or would you prefer if I change it to SimpleExitableScreen and labels referencing code editor would be passed as props? If we don't keep CodeEditorScreen on the Interface package what would be the ideal place to keep it?
Regarding CodeEditor, another alternative instead of exposing is to change wp.editor.PostTextEditor that we already have to also accept value, onChange, onInput. When the props are not passed it would behave exactly as it does now and retrieve them assuming what is being edited is the current post if the props are passed it works in a generic way.
Do we prefer an update to wp.editor.PostTextEditor or expose a new component under wp.components?
There was a problem hiding this comment.
Gotcha, also makes sense. The separate package could maybe make sense if we feel that the code editor is too specific to live in the general-purpose @wordpress/components package, as Jorge suggested.
There was a problem hiding this comment.
The component should look mostly equal in both places and I think repeating hundreds of lines of code that we need to change in two places each time the UI changes is not a good idea.
When did the last time the UI changed?
The interface package was a simple solution it is private can be used by edit-post and edit site.
We had this issue for all the UI of the screens so far, and every time we try to share code, personally, it feels wrong to me because we just share code to share code and we don't think about the right abstractions/packages and cripple them.
--
I understand that it's just easy for developers and don't repeat yourself and and all of that but I can't stop but feel that it's wrong. We don't make the necessary efforts to build the right abstraction and we just want to ship our code. For me if the goal is to just ship the feature, it's better to duplicate. If we want to share we should start thinking about the right abstraction, the right packages.
I want to use the interface package today, it became so crippled that I won't ever look for it because it's not clear what is it for, it just there for convenience which is sad. I guess we can continue that trend because it's already a very bad package. I won't block this anymore.
There was a problem hiding this comment.
A long time ago, I proposed that editSite is just editPost with another function to initialize it. I think that would have been the best solution but probably too late now.
We may consider creating another private package to put artifacts reused between editor, edit-post, and edit-site that we don't think are something worth exposing (maybe named editor-utils?).
I think that's what "interface" has become, we can either continue the trend or start over and try to clean "interface" and add a new package because make it clear that it's just to share code between edit-post and edit-site and ideally shouldn't exist.
There was a problem hiding this comment.
Hi @youknowriad, I did not have a strong preference for extending the interface package. I updated the PR to not add anything to the interface package and follow the duplication approach.
A long time ago, I proposed that editSite is just editPost with another function to initialize it. I think that would have been the best solution but probably too late now.
This would have been the right approach. Sharing functionality between edit-site and edit-post is always hard while their UI is mostly the same. But I guess now we can not make them a single package.
The interface package already says "Interface module for WordPress. The package contains shared functionality across the modern JavaScript-based WordPress screens." so the idea of the package is already to share functionality between the screens and the package exists just for that but we still have a higher standard of what should be an abstraction there vs what is a component when only internal to a package and the current components there are more generic than what we had here and contain readme files etc.
jorgefilipecosta
force-pushed
the
add/code-editor-to-edit-site
branch
2 times, most recently
from
08587e8
to
8c78433
Compare
jorgefilipecosta
force-pushed
the
add/code-editor-to-edit-site
branch
from
8c78433
to
b9a4c6c
Compare
Done 👍 |
Part of #21245.
Fixes: #22528
This PR implements the code editor for the edit-site screen.
In order to not repeat all the code and logic between edit-post and edit-site two generic components were added to the WordPress/interface package: CodeEditor and CodeEditorScreen. The edit post was refactored to use the new components.
How has this been tested?
I verified I can use the code editor on the edit site.
I verified I can switch between code and visual editors using the keyboard shortcut.
I changed some blocks on the code editor switched to the visual editor and verified the changes were reflected.
I changed some blocks on the code editor without removing focus from the textarea saved the template using the keyboard shortcut and verified the changes were saved.
I verified I could use the code editor with template parts.
Screenshots