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.

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

Editor: Make revisions more prominent #62323

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 30 additions & 7 deletions packages/editor/src/components/post-last-revision/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +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() {
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

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

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

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

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

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

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 @@ -47,4 +52,22 @@ function PostLastRevision() {
);
}

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;
4 changes: 4 additions & 0 deletions packages/editor/src/components/post-last-revision/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@
padding: $grid-unit-20;
}
}

.editor-private-post-last-revision__button {
display: inline-block;
}
2 changes: 2 additions & 0 deletions packages/editor/src/components/sidebar/post-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import PostsPerPage from '../posts-per-page';
import SiteDiscussion from '../site-discussion';
import { store as editorStore } from '../../store';
import TemplateAreas from '../template-areas';
import { PrivatePostLastRevision } from '../post-last-revision';

/**
* Module Constants
Expand Down Expand Up @@ -76,6 +77,7 @@ export default function PostSummary( { onActionPerformed } ) {
<PostAuthorPanel />
<PostTemplatePanel />
<PostDiscussionPanel />
<PrivatePostLastRevision />
<PageAttributesPanel />
<PostSyncStatus />
<BlogTitle />
Expand Down
Loading