Reorganize folders within editor #408

mtias · 2017-04-12T10:07:44Z

Moving general components out of it, and flattening structure more.

mtias · 2017-04-12T10:11:12Z

@aduth @youknowriad pushing a rough PR that expands on #399.

nylen

In general one of the things that makes development on this project pretty hard to follow is frequent renaming of files and components. I agree it makes more sense to do this sooner rather than later, but I'm left wondering how much more there is to do and whether we have an overall plan for how we want things to look.

nylen · 2017-04-12T11:24:16Z

components/block-toolbar/index.js

@@ -7,7 +7,7 @@ import classNames from 'classnames';
 * Internal dependencies
 */
 import './style.scss';
-import Dashicon from 'components/dashicon';
+import Dashicon from 'dashicon';

 function Toolbar( { controls } ) {


Here and elsewhere, if we're changing the filename the component name should also change (to BlockToolbar in this case).

nylen · 2017-04-12T11:24:58Z

editor/header/mode-switcher/index.js

@@ -7,7 +7,7 @@ import { connect } from 'react-redux';
 * Internal dependencies
 */
 import './style.scss';
-import Dashicon from 'components/dashicon';
+import Dashicon from '../../../components/dashicon';


I'm pretty sure this should still just be components/dashicon - see #386.

nylen · 2017-04-12T11:25:15Z

editor/header/saved-state/index.js

@@ -2,7 +2,7 @@
 * Internal dependencies
 */
 import './style.scss';
-import Dashicon from '../../components/dashicon';
+import Dashicon from '../../../components/dashicon';


Same, we shouldn't need these relative paths any more.

nylen · 2017-04-12T11:25:24Z

editor/header/tools/index.js

+import Dashicon from '../../../components/dashicon';
+import IconButton from '../../../components/icon-button';
+import Inserter from 'inserter';
+import Button from '../../../components/button';


Same, we shouldn't need these relative paths any more. If I'm wrong about that, then it seems like we need to reconsider how the import paths work.

I was having issues with the entry points, but I also put this together very roughly to communicate the structure.

nylen · 2017-04-12T11:26:24Z

editor/visual-editor/block.js

@@ -7,7 +7,7 @@ import classnames from 'classnames';
 /**
 * Internal dependencies
 */
-import Toolbar from 'components/toolbar';
+import Toolbar from '../../components/block-toolbar';


Should be BlockToolbar here and in other usage as well.

nylen · 2017-04-12T11:27:15Z

components/block-toolbar/index.js

@@ -7,7 +7,7 @@ import classNames from 'classnames';
 * Internal dependencies
 */
 import './style.scss';
-import Dashicon from 'components/dashicon';
+import Dashicon from 'dashicon';


Shouldn't dashicon still be under components?

aduth · 2017-04-12T12:32:50Z

I'd like to see some rationalization for these changes.

As configured currently, each of the top-level entry points is intentionally isolated from the others. This does lead to awkward consequences such as two separate components directories in editor and in blocks, but it is reflective of more of a final API where we fully expect plugin authors to need to incorporate Editable into their blocks implementations. How would a plugin author gain access to this component in this restructuring? Are all components in the components directory to be made available external to the editor? What's the criteria for including a component in the components directory? Will a flattened editor directory scale?

mtias · 2017-04-12T12:35:45Z

Are all components in the components directory to be made available external to the editor?

The root level components directory is meant to hold generic components (button, dashicon, button-icon, block-toolbar, etc) that are not exclusive to the editor.

Will a flattened editor directory scale?

I think it should, but we'll see as it grows. What do you imagine would prevent it from scaling?

aduth · 2017-04-12T13:09:43Z

The root level components directory is meant to hold generic components (button, dashicon, button-icon, block-toolbar, etc) that are not exclusive to the editor.

Are they exclusive to the editor though? I am very much curious to see specific examples of how these components would be consumed, both within this project (ideally not relative paths) but more importantly from an external plugin.

What do you imagine would prevent it from scaling?

This might depend on how we classify components as general-purpose. Is the Inserter general purpose? Maybe not, but I also don't think BlockToolbar is either. How do we decide when to nest components in other components? The current approach is somewhat "pragmatic" but loosely related by region, and the path to upgrading a component as being relevant to more than one region is fairly clear (move to editor/components, #403).

mtias · 2017-04-12T13:15:16Z

Right now I'm thinking of blocks using them for their UI, the editor using it for the chrome, and other areas of the admin using it in place of html-elements-with-classes as they move to be powered by JS more, and hopefully our element abstraction.

Is the Inserter general purpose? Maybe not, but I also don't think BlockToolbar is either.

The inserter, for now, seems specific to the editor. Though I can see it becoming more generic and placed within blocks. The block-toolbar is just block UI, no? Why do you think it's connected with the editor?

aduth · 2017-04-12T13:21:50Z

The block-toolbar is just block UI, no? Why do you think it's connected with the editor?

In my mind I'm thinking of general purpose as providing usefulness outside the Gutenberg project as a whole, since within the project we won't expect anything except those files within editor to actually perform rendering.

mtias · 2017-04-13T10:56:29Z

Based on https://wpcoredesign.mystagingwebsite.com/gutenberg/#blueprints

General components:

button.
dashicon.
icon-button.

Block specific:

block-mover.
block-switcher.
block-toolbar.

Editor specific:

editor-bar.
inserter.

youknowriad · 2017-04-13T12:19:49Z

@aduth One issue we have if we leave the Editable in the blocks module separated from the other components is that we can not use the other components to build it. For example, we can not use dashicon to build the formatting toolbar.

aduth · 2017-04-13T13:01:15Z

@youknowriad It's a good point to raise. Part of this might depend on whether the inline formatting behaves as an inline toolbar (popup) or part of the selected block's bounding toolbar. In the latter case, this seems more the responsibility of the editor to render and instead the Editable should merely be communicating intent for it to be shown.

youknowriad · 2017-04-19T10:42:54Z

I think we should move forward with this. I'm thinking more and more that having a single global module is the way to go. The current folder structure has limits. We can not use components like "dashicon" or "inserter" inside a block or the Editable.

For #447 we may use the inserter inside the Editable, we can also use the Inserter inside the text block when the text block is empty.

As mentioned above, the Editable will include a toolbar (needs dashicon) ...

And think use-cases like these, we'll have a lot.

aduth · 2017-04-19T13:21:28Z

I'm not opposed to this. It's just a little unclear to me still what folders we'll have, which components go where, and how they're exposed in the global scope for plugins.

Do we keep blocks/components ? editor/components ?
Are we comfortable with all components within the components being exposed on the global scope. What's the naming? wp.components.IconButton ?

I'm thinking more and more that having a single global module is the way to go.

I think it's reasonable to consider it a single module already. Even if we created a src directory, we'd still likely have a structure not too different than what already exists in the root. As far as sharing components between the different directories like in your example of block's Editable, a root-level components directory would be fine, but to the questions above, I don't know that we can just move all components here without some consideration for how they're accessed externally.

nylen · 2017-04-19T19:28:55Z

I don't know that we can just move all components here without some consideration for how they're accessed externally.

I agree with this. I think we should be very selective about which components we make public, defaulting to hiding things until they are needed. Related issues have arisen with the media library JS bundle already in core: https://core.trac.wordpress.org/changeset/31935

mtias · 2017-04-26T16:32:53Z

Let's take some time to revisit this after we wrap the prototype-parity-milestone next week.

mtias added 3 commits April 12, 2017 11:32

Move "components" to top level. Reorganize editor specific ones.

fe70e90

Move "modes" to root of editor.

8993b67

Update import paths.

251adb3

mtias added the Framework Issues related to broader framework topics, especially as it relates to javascript label Apr 12, 2017

mtias requested review from youknowriad and aduth April 12, 2017 10:07

nylen requested changes Apr 12, 2017

View reviewed changes

mtias mentioned this pull request Apr 13, 2017

Visual Editor: Adding the arrangement toolbar (up/down arrows) #412

Merged

aduth mentioned this pull request Apr 17, 2017

Visual Editor: Keep track of the focused block #438

Merged

aduth mentioned this pull request Apr 25, 2017

Try Slot & Fill pattern for rendering Editable formatting controls #507

Merged

aduth mentioned this pull request Apr 27, 2017

Try avoiding parse to get content of Editable #523

Merged

mtias closed this May 3, 2017

ntwb deleted the update/reorg branch August 5, 2017 01:21

snyk-bot mentioned this pull request Jan 15, 2022

[Snyk] Security upgrade markdownlint from 0.18.0 to 0.25.1 Verdinjoshua26/gutenberg#27

Open

Verdinjoshua26 mentioned this pull request Sep 25, 2022

[Snyk] Fix for 16 vulnerabilities Verdinjoshua26/gutenberg#50

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize folders within editor #408

Reorganize folders within editor #408

mtias commented Apr 12, 2017

mtias commented Apr 12, 2017

nylen left a comment

nylen Apr 12, 2017

nylen Apr 12, 2017

nylen Apr 12, 2017

nylen Apr 12, 2017

mtias Apr 12, 2017

nylen Apr 12, 2017

nylen Apr 12, 2017

aduth commented Apr 12, 2017

mtias commented Apr 12, 2017

aduth commented Apr 12, 2017

mtias commented Apr 12, 2017

aduth commented Apr 12, 2017

mtias commented Apr 13, 2017

youknowriad commented Apr 13, 2017

aduth commented Apr 13, 2017

youknowriad commented Apr 19, 2017

aduth commented Apr 19, 2017

nylen commented Apr 19, 2017 •

edited

Loading

mtias commented Apr 26, 2017

Reorganize folders within editor #408

Reorganize folders within editor #408

Conversation

mtias commented Apr 12, 2017

mtias commented Apr 12, 2017

nylen left a comment

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

mtias Apr 12, 2017

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

nylen Apr 12, 2017

Choose a reason for hiding this comment

aduth commented Apr 12, 2017

mtias commented Apr 12, 2017

aduth commented Apr 12, 2017

mtias commented Apr 12, 2017

aduth commented Apr 12, 2017

mtias commented Apr 13, 2017

youknowriad commented Apr 13, 2017

aduth commented Apr 13, 2017

youknowriad commented Apr 19, 2017

aduth commented Apr 19, 2017

nylen commented Apr 19, 2017 • edited Loading

mtias commented Apr 26, 2017

nylen commented Apr 19, 2017 •

edited

Loading