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

Jump to bottom

Site Editor: Add page details when viewing a specific page. #48650

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Site Editor: Add page details when viewing a specific page. #48650

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ded978a
Show the post link
scruffian Mar 1, 2023
7e5d717
Open in new window
scruffian Mar 1, 2023
d8e83b4
Use ExternalLink component for link
jeryj Mar 1, 2023
6159945
Style link to use gray color
jeryj Mar 1, 2023
bb56064
fix the grid unit variable name
scruffian Mar 1, 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
19 changes: 17 additions & 2 deletions packages/edit-site/src/components/sidebar-navigation-screen-navigation-item/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
*/
import { __ } from '@wordpress/i18n';
import { useDispatch, useSelect } from '@wordpress/data';
import { __experimentalUseNavigator as useNavigator } from '@wordpress/components';
import {
__experimentalUseNavigator as useNavigator,
ExternalLink,
} from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { decodeEntities } from '@wordpress/html-entities';
import { pencil } from '@wordpress/icons';
Expand Down Expand Up @@ -49,7 +52,19 @@ export default function SidebarNavigationScreenNavigationItem() {
/>
}
content={
post ? decodeEntities( post?.description?.rendered ) : null
<>
{ post?.link ? (
<ExternalLink
className="edit-site-sidebar-navigation-screen__page-link"
href={ post.link }
scruffian marked this conversation as resolved.
Show resolved Hide resolved
>
scruffian marked this conversation as resolved.
Show resolved Hide resolved
{ post.link }
</ExternalLink>
) : null }
{ post
? decodeEntities( post?.description?.rendered )
: null }
</>
}
/>
);
Expand Down
13 changes: 13 additions & 0 deletions packages/edit-site/src/components/sidebar-navigation-screen/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@
color: $gray-600;
}

.edit-site-sidebar-navigation-screen__page-link {
color: $gray-600;

&:hover,
&:focus {
color: $white;
}

.components-external-link__icon {
margin-left: $grid-unit-5;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't build for me, I get:

'Error: Undefined variable.\n' +
    '   ╷\n' +
    '22 │         margin-left: $grid-unit-5;\n' +
    '   │                      ^^^^^^^^^^^^\n' +
    '   ╵\n' +

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a fix for this

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'm surprised this isn't part of the component itself. It might be worth looking into a more general fix for the component, in a followup PR...

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 surprised this isn't part of the component itself. It might be worth looking into a more general fix for the component, in a followup PR...

I think it's cause this can be used in smaller inline text, where its not necessary to add space before the icon. Although we probably should have some space after the icon in the screenshot:

CleanShot 2023-03-01 at 17 47 22

Copy link
Member

Choose a reason for hiding this comment

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

Pushed a fix for this

Did you push it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops no, I have now!

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird — it both built and showed a space as if it was working for me 🤷🏻 Thanks for catching and fixing that!

}
}

.edit-site-sidebar-navigation-screen__title-icon {
position: sticky;
top: 0;
Expand Down