Skip to content
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.

Sign up for GitHub

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

List view: update to remember lastest state rather than needing a preference #50564

Closed
wants to merge 2 commits into from
Closed

List view: update to remember lastest state rather than needing a preference #50564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b144b80
Make the list view remember its open state rather than needing a pref…
glendaviesnz May 12, 2023
9926086
Also add to site editor
glendaviesnz May 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/edit-post/src/components/header/header-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const preventDefault = ( event ) => {

function HeaderToolbar() {
const inserterButton = useRef();
const { setIsInserterOpened, setIsListViewOpened } =
const { setIsInserterOpened, setIsListViewOpened, toggleFeature } =
useDispatch( editPostStore );
const {
isInserterEnabled,
Expand Down Expand Up @@ -85,10 +85,10 @@ function HeaderToolbar() {
/* translators: accessibility text for the editor toolbar */
const toolbarAriaLabel = __( 'Document tools' );

const toggleListView = useCallback(
() => setIsListViewOpened( ! isListViewOpen ),
[ setIsListViewOpened, isListViewOpen ]
);
const toggleListView = useCallback( () => {
toggleFeature( 'showListViewByDefault' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well toggleFeature means, this is still a preference. The only difference is that the preference is not used for the "initial state" but for the "latest state". I think that's not great because it doesn't behave like a preference and because it triggers an API request each time you open/close the list view.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay in replying, this one slipped off my list. Yeh, the use of preference here is confusing. If anything it should work the same as the pinned items approach used for the right panels.

Can you see any reason why the list view shouldn't work the same as the right inspector panel, ie. it doesn't have any preference, it just remembers its last state? /cc @jameskoster

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you see any reason why the list view shouldn't work the same as the right inspector panel

Not really. The 'remembering' seems more useful / consistent.

Iirc the preference was originally created so that third parties can force List View to be open initially on new sites. As long as there is a way to handle that programmatically, then it would seem reasonable to remove the preference once the remembering is added.

setIsListViewOpened( ! isListViewOpen );
}, [ toggleFeature, setIsListViewOpened, isListViewOpen ] );
const overflowItems = (
<>
<ToolbarItem
Expand Down
7 changes: 0 additions & 7 deletions packages/edit-post/src/components/preferences-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,6 @@ export default function EditPostPreferencesModal() {
'Show text instead of icons on buttons.'
) }
/>
<EnableFeature
featureName="showListViewByDefault"
help={ __(
'Opens the block list view sidebar by default.'
) }
label={ __( 'Always open list view' ) }
/>
<EnableFeature
featureName="themeStyles"
help={ __(
Expand Down
9 changes: 5 additions & 4 deletions packages/edit-site/src/components/header-edit-mode/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export default function HeaderEditMode() {
setIsListViewOpened,
} = useDispatch( editSiteStore );
const { __unstableSetEditorMode } = useDispatch( blockEditorStore );
const { toggle } = useDispatch( preferencesStore );

const isLargeViewport = useViewportMatch( 'medium' );

Expand All @@ -121,10 +122,10 @@ export default function HeaderEditMode() {
}
}, [ isInserterOpen, setIsInserterOpened ] );

const toggleListView = useCallback(
() => setIsListViewOpened( ! isListViewOpen ),
[ setIsListViewOpened, isListViewOpen ]
);
const toggleListView = useCallback( () => {
toggle( 'core/edit-site', 'showListViewByDefault' );
setIsListViewOpened( ! isListViewOpen );
}, [ toggle, setIsListViewOpened, isListViewOpen ] );

const { useShouldContextualToolbarShow } = unlock( blockEditorPrivateApis );
const {
Expand Down
7 changes: 0 additions & 7 deletions packages/edit-site/src/components/preferences-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ export default function EditSitePreferencesModal( {
label={ __( 'Show button text labels' ) }
help={ __( 'Show text instead of icons on buttons.' ) }
/>
<EnableFeature
featureName="showListViewByDefault"
help={ __(
'Opens the block list view sidebar by default.'
) }
label={ __( 'Always open list view' ) }
/>
<EnableFeature
featureName="showBlockBreadcrumbs"
help={ __(
Expand Down