Skip to content

Editor: Make revisions more prominent #62323

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

Merged
ntsekouras merged 2 commits into from
Jun 10, 2024
Merged

Editor: Make revisions more prominent #62323

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abca5d0
Editor: Make revisions more prominent
ntsekouras Jun 5, 2024
97d22df
use panel row
ntsekouras Jun 6, 2024
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
Prev Previous commit
use panel row
  • Loading branch information
@ntsekouras
ntsekouras committed Jun 7, 2024
commit 97d22df8bb30cf006a40c893bea38eec63fe8aee
44 changes: 31 additions & 13 deletions packages/editor/src/components/post-last-revision/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,27 @@ import { addQueryArgs } from '@wordpress/url';
* Internal dependencies
*/
import PostLastRevisionCheck from './check';
import PostPanelRow from '../post-panel-row';
import { store as editorStore } from '../../store';

/**
* Renders the component for displaying the last revision of a post.
*
* @return {Component} The component to be rendered.
*/
function PostLastRevision() {
return <PrivatePostLastRevision isLink={ false } />;
}

export function PrivatePostLastRevision( { isLink } ) {
const { lastRevisionId, revisionsCount } = useSelect( ( select ) => {
function usePostLastRevisionInfo() {
return useSelect( ( select ) => {
const { getCurrentPostLastRevisionId, getCurrentPostRevisionsCount } =
select( editorStore );
return {
lastRevisionId: getCurrentPostLastRevisionId(),
revisionsCount: getCurrentPostRevisionsCount(),
};
}, [] );
}

/**
* Renders the component for displaying the last revision of a post.
*
* @return {Component} The component to be rendered.
*/
function PostLastRevision() {
Copy link
Member

@Mamaduka Mamaduka Jun 7, 2024

Choose a reason for hiding this comment

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

I'm looking at some other panel refactorings; they didn't keep the original versions. Example: #61357.

Should we do the same here?

Copy link
Contributor Author

@ntsekouras ntsekouras Jun 7, 2024

Choose a reason for hiding this comment

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

I don't think it's the same case because here we're not changing the entire panel (PostLastRevisionPanel) that is exported. So consumers of the above panel, should have the 'old' PostLastRevision component.

Copy link
Member

@Mamaduka Mamaduka Jun 7, 2024

Choose a reason for hiding this comment

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

If the policy is to keep the old components intact, then we might need to revisit some of the refactoring, like #61357. Because PostDiscussionPanel has a new structure.

Copy link
Contributor Author

@ntsekouras ntsekouras Jun 7, 2024

Choose a reason for hiding this comment

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

I don't think there is a clear policy about that, but I think the changes should not be disruptive in the sense that they would make sense in other designs too. An example would be if we change a control and the exporting component is just that. In this case IMO it's fine to not keep the old version. In this PR for me it made more sense to keep the old one as it matches better when used inside PostLastRevisionPanel with the panel body and we actually don't use PostLastRevisionPanel anymore in our codebase.

Can you share the components you're concerned with the discussions panel? Also cc @jorgefilipecosta for visibility.

Copy link
Member

@Mamaduka Mamaduka Jun 7, 2024

Choose a reason for hiding this comment

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

My concern is that the discussion panel consumers expect it to render PanelBody > PanelRow. Now, they'll get a different design.

I personally don't mind either path for changes as long as they're consistent.

Copy link
Contributor Author

@ntsekouras ntsekouras Jun 7, 2024

Choose a reason for hiding this comment

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

I know they get a different design and might be a bit different, but in my mind they still get the whole functionality (the whole panel). If I had to update the panel here for revisions personally I'd follow the same approach with the discussions panel. This is subjective though..

const { lastRevisionId, revisionsCount } = usePostLastRevisionInfo();

return (
<PostLastRevisionCheck>
Expand All @@ -39,17 +40,34 @@ export function PrivatePostLastRevision( { isLink } ) {
revision: lastRevisionId,
} ) }
className="editor-post-last-revision__title"
icon={ isLink ? undefined : backup }
icon={ backup }
iconPosition="right"
text={ sprintf(
/* translators: %s: number of revisions */
__( 'Revisions (%s)' ),
revisionsCount
) }
variant={ isLink ? 'link' : undefined }
/>
</PostLastRevisionCheck>
);
}

export function PrivatePostLastRevision() {
const { lastRevisionId, revisionsCount } = usePostLastRevisionInfo();
return (
<PostLastRevisionCheck>
<PostPanelRow label={ __( 'Revisions' ) }>
<Button
href={ addQueryArgs( 'revision.php', {
revision: lastRevisionId,
} ) }
className="editor-private-post-last-revision__button"
text={ revisionsCount }
variant="tertiary"
/>
</PostPanelRow>
</PostLastRevisionCheck>
);
}

export default PostLastRevision;
9 changes: 4 additions & 5 deletions packages/editor/src/components/post-last-revision/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
.editor-post-last-revision__title {
width: 100%;
font-weight: 500;

&.is-link {
width: fit-content;
font-weight: unset;
}
}

.editor-post-last-revision__title.components-button.has-icon {
Expand Down Expand Up @@ -33,3 +28,7 @@
padding: $grid-unit-20;
}
}

.editor-private-post-last-revision__button {
display: inline-block;
}
2 changes: 1 addition & 1 deletion packages/editor/src/components/sidebar/post-summary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export default function PostSummary( { onActionPerformed } ) {
<VStack spacing={ 1 }>
<PostContentInformation />
<PostLastEditedPanel />
<PrivatePostLastRevision isLink />
</VStack>
{ ! isRemovedPostStatusPanel && (
<VStack spacing={ 2 }>
Expand All @@ -78,6 +77,7 @@ export default function PostSummary( { onActionPerformed } ) {
<PostAuthorPanel />
<PostTemplatePanel />
<PostDiscussionPanel />
<PrivatePostLastRevision />
<PageAttributesPanel />
<PostSyncStatus />
<BlogTitle />
Expand Down