-
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
Reorganize the site editor to introduce Browse Mode. #44770
Conversation
There's more than enough for multiple developers in this PR, if you want to do a sub PR to solve one of the todo items, please let us know. |
Size Change: -1.17 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
This is really exciting to see, thanks so much for working on this! Even in this early state, it feels like there are benefits to have with this structure: not just being dumped into editing a specific template, but letting you choose one first, comes to mind. One small nitpick off the top: I'd keep the dark-background W inside the fullscreen editor and have it remain a fixed anchor across everything. This is an older prototype showing the interaction, but as you can see it's keeping the W fixed even across animations. |
What would the "W" button do, when the sidebar is open? |
One of the ideas for later is that it would pull you out of the "appearance" menu and send you to the root of the admin. |
For me, that is a suggestion that the button in the sidebar and the one in the header of "edit" mode are actually two completely different things. I think the button in the header of "edit" mode and the "edit" button in the "view" mode are the same thing though. So despite their content change (logo or "edit" text), semantically that button is the same thing, it's just a button to toggle between "edit" and "view" mode. Also by keeping that button as a single one to toggle between both, we solve potential "focus loss" issue because the focus would just stay in the button itself. Now, that doesn't mean we can try to do something visually close to the mockup where "it" feels like the button doesn't move. |
I wouldn't say that it's semantically a different button, it just has contextual behaviours like the home button used to have on iphones, but it's the same element. |
fce010d
to
3ca4bb4
Compare
I just wanted to note that this change will probably break block-based template parts for the Classic theme. The current implementation relies on the dedicated page list. Not a blocker, but something to keep in mind :) |
Exciting PR! :D
I don't know if this was decided upon separately but I'm not sure its the best approach. A 'dedicated' list view will likely be needed to handle bulk actions. If we have both, the flows can get a bit awkward. An alternative direction would be to keep the list view(s) and provide access to them via the Navigator like so: templates.mp4From here we can expand to include the aforementioned bulk actions. It also provides a consistent destination for the W button when editing a template – back to the full templates list. This aligns with the Post / Page editor. I assume it's part of the "Finish the "view" mode" item, but I just wanted to note that if you have the Inspector or Global Styles open in edit mode, they remain open in view mode: |
3ca4bb4
to
afd99e4
Compare
@jameskoster I think we went too far with the full canvas management view for templates as the default and we need to bring back the quick access in the sidebar. For most operations — selecting a template and creating a new one — the sidebar is the fastest and least disorienting route. We should keep the full list view available for more advanced operations, integrations with plugin bundles, listing all customized templates (from all themes), etc. In a similar way how we still want to get quick access to pages but it doesn't replace the pages admin management view. |
Ok, let's keep the dedicated views for now, and we can iterate on the navigation sidebar more easily now. Let's just try to find the minimum acceptable changes to be done in this PR. |
packages/edit-site/src/components/sidebar-navigation-title/index.js
Outdated
Show resolved
Hide resolved
Okay, more detailed explorations can take place in #37001. |
@Mamaduka Do you know if this is something that is covered by an e2e test. I think it would be great to add one if not. |
@youknowriad, I'll try and add a simple e2e test case 👍 I can see that we're keeping the template list view, so that solves my main concern :) |
packages/components/src/navigator/navigator-screen/component.tsx
Outdated
Show resolved
Hide resolved
It definitely is an exciting PR, but is there a way to avoid shipping this in one big PR? I've been contributing for a while now, and I notice every time we do this it results in a tonne of regressions. No matter how hard contributors try to verify there's no breakage, there's always a lot. G2 was like that. Popover refactor to floating-ui was like that (@ciampo, @ntsekouras and I were fixing bugs for months), and a lot of other bigger refactors have been like that. It is a huge time killer. When fixing popover bugs I personally had very little time to work on features as I wanted to make sure there weren't major bugs in 6.1—either that or reverting the refactor were the only solutions, but by that time a revert was already complicated. I think it'd be really good if we could try breaking this into smaller more reviewable chunks, potentially behind experimental feature toggles if those features can't be released separately during the development process. This PR can act as a spike/exploration, but not something that's merged. |
packages/edit-site/src/components/sidebar-navigation-item/index.js
Outdated
Show resolved
Hide resolved
Took this for a quick spin, and it's coming together so fast, nice! A few small notes:
|
a94be4c
to
a8683bd
Compare
Thanks all for your help on this PR, this is only the first step, I added a list of todo list items to the parent issue #36667 |
Co-authored-by: James Koster <james@jameskoster.co.uk> Co-authored-by: Matías Ventura <mv@matiasventura.com>
Looks like this PR broke the ARIA landmarks structure and the |
@afercia Thanks for the notice, I'll be taking a look. |
See #36667 (Discussion and review in #45346)
What?
This is a base branch to the on-going work to redo the navigation in the site editor. Target is something close to this
browse.mp4
This is going to be big PR. I'm opening it soon with a lot of todo items so we can see how to split up the work. I think the PR can be landed when all the current features of the site editor are restored properly without any breakage. Once the new edit-site organization is restored, we can work on adding the browse mode to the header in view mode as a first step for "Browse Mode".
I've added a lot of todo comments in the code base in order to keep track of the remaining work, here's the summary of the tasks that we need to solve.
/new/
folder and move the components back to thecomponents
folder.__experimentalMainDashboardButton