-
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
Move document information and outline to list view panel #44788
Move document information and outline to list view panel #44788
Conversation
Size Change: +2.75 kB (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
0b733a2
to
56f5c2d
Compare
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 appears to have a bug in Firefox:
transparent-stats.mp4
56f5c2d
to
8f39ab3
Compare
Nice catch @draganescu the issue was fixed 👍 |
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.
Lovely 👏🏻 This looks great now. Are these two tweaks worth a second look?
- there is a jankiness when toggling inserter and list view
video
janky-inserter-listview.mp4
- there is a page jump when toggling document structure and document outline
video
jump-structure-outline.mp4
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.
There is a missing feature from the original issue (#44193):
Outline tab only available if the document has heading blocks
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.
@jorgefilipecosta Thanks for the PR. This needs some accessibility work, please see comments below.
packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js
Show resolved
Hide resolved
packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js
Show resolved
Hide resolved
packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js
Outdated
Show resolved
Hide resolved
b9ea0e8
to
6f6fde8
Compare
Hi @draganescu, @alexstine, thank you a lot for the reviews, I tried to apply all the feedback. |
@jorgefilipecosta Why does this info appear on List View and Outline tabs? Shouldn't it just appear on Outline tab?
This information is not expected in a List View tab especially since it appears after an application role. |
Hi @alexstine; thank you for letting me know about this issue. |
@jorgefilipecosta Not really. You are dealing with a contextual issue. When the designers opened the door with tabs, you set the principal that there would be very defined content per tab. E.g. If you have a list view tab, this is okay, but that tab should only contain list view related content. The outline tab would make much more sense to find this information under and it would be more consistent with the idea of one defined item per tab. Currently, user focus is placed in the application when the list view opens so users would have to know that other content is there to find it. You cannot add this info to the application because this is what makes the tree grid items accessible. Adding another role could actually make things inaccessible because of the very nature of how screen readers are choosing to work now. TLDR: No. Thanks. |
@mtias, @jasmussen, @jameskoster given the acessibiity contraints referred above would it be ok to have character word/count just on the outline tab? |
That seems reasonable to me, indeed we discussed on the original ticket. It does mean we'll need to display the Overview tab permanently though (so stats are always accessible). Consequently the Overview tab will need an 'empty' design... for now it can probably just contain a description. |
Hey @alexstine !
I'm not not sure I understand what you meant here, would you mind clarifying?
Again, I'm not sure I understand the issue that you're pointing out. Looking at the
Using space/enter is indeed another variant of the Tabs pattern, described as "Tabs with manual activation" on the same W3C Website, which currently the What would be the exact reason for using manual activation over automatic activation? Once these requirements are a bit more clear, we may open a PR and discuss the next steps for the
While it is true that a good part of the development work in Gutenberg goes towards adding new features and evolving the product, I can guarantee you that at the same time we try to do our best work in constantly polishing and improving our code. Folks like @mirka and myself, for example, spend most of their time improving the Finally, I'm not sure I'd call the |
@ciampo Okay, I guess I should have had my coffee before writing this. Let me explain.
As far as the List View and Outline tabs, they are the same as the document settings/block settings sidebar. Those are perfectly accessible, that is why the PR was okay to me. It follows a model already used in the editors.
We cannot use automatic activation here because the As far as the mentioned prop, this would not need to be changed globally. This is a case-by-case thing. That is why I suggested adding a new prop to
The I know you work hard to improve but at the same time, know that the vast majority of PRs I review for A11Y are new features/changing UIs. There should be a larger focus on improving what we already have, that was my point for the project as a whole, not specific to your great work on components. What good are accessible components if they are used in inaccessible contexts. That is what I am fighting to improve. Sorry my above comment came off so rough but I meant nothing against the contributors over the components team. That feedback was for this PR alone. Thanks. |
Thank you for the context and the detailed explanation, @alexstine ! I went ahead and opened a new issue related to adding support for manual tab activation to
Don't worry about it! As I mentioned before, we always appreciate your contributions and your will to improve the accessibility of the editor. |
The "Details" popover doesn't exist since v14.5. See WordPress/gutenberg#44788
* Fix tracking specs against GB v14.8.x * Fix editor-tracking__other-block-events.ts * Fix specs/editor-tracking/editor-tracking__global-styles-events.ts * Remove accidental skip * More global styles tracking fixes * Remove "Details" popover test & related actions The "Details" popover doesn't exist since v14.5. See WordPress/gutenberg#44788 * Fix editor-tracking__toolbar-events.ts * Fix editor-tracking__document-actions-events.ts * Fix editor-tracking__fse-template-events.ts * Fix editor-tracking__pattern-events.ts * Fix padding setting in editor-tracking__global-styles-events.ts * Remove accidental page.close * Update block names for template conversion * Update template part deletion for new site editor * Move page closing to a dedicated step As per #71572 (comment) * Update test to match current theme (twenty-twenty-two) * Skip the whole block since currently tracking is not tested there * Redesign spec around 2022 theme Co-authored-by: dpasque <dan.speckhard.pasque@automattic.com>
* Fix tracking specs against GB v14.8.x * Fix editor-tracking__other-block-events.ts * Fix specs/editor-tracking/editor-tracking__global-styles-events.ts * Remove accidental skip * More global styles tracking fixes * Remove "Details" popover test & related actions The "Details" popover doesn't exist since v14.5. See WordPress/gutenberg#44788 * Fix editor-tracking__toolbar-events.ts * Fix editor-tracking__document-actions-events.ts * Fix editor-tracking__fse-template-events.ts * Fix editor-tracking__pattern-events.ts * Fix padding setting in editor-tracking__global-styles-events.ts * Remove accidental page.close * Update block names for template conversion * Update template part deletion for new site editor * Move page closing to a dedicated step As per #71572 (comment) * Update test to match current theme (twenty-twenty-two) * Skip the whole block since currently tracking is not tested there * Redesign spec around 2022 theme Co-authored-by: dpasque <dan.speckhard.pasque@automattic.com>
Fixes: #44193
Fixes: #44862
Moves document information and outline to the list view panel on the edit post screen, trying to follow the mockups proposed in #44193.
Screenshots or screencast