Add a 'View Site' link in the site editor #42331
Conversation
jameskoster
added
the
[Feature] Site Editor
|
Size Change: +57 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
I love this improvement. I just saw one thing: In the post editor this produces the following redundancy:
|
|
Oops 😅 I'll fix that when I get a moment. |
|
OK I changed the implementation a bit so that the 'View Site' link only appears in the site editor. Updating the button label to 'View' in the Site Editor makes sense, I'm not 100% sure about the post editor though. There's value in consistency, and I maintain the viewpoint that the device switchers aren't strictly just previews, but the post editor does include a dedicated preview link. What do you think? |
|
Thank you James!! This looks really good! I have been looking forward to seeing a View page drop down in FSE for quite a while. Should another PR change the wording in a page and post? |
|
Just noting that this came up as something that would be welcomed in the FSE Outreach Program's fifteenth call for testing:
In this case, the view site would happen post save flow, which to me is the best of both worlds since it helps avoid the confusion that might come from making changes, viewing site, and not seeing those changes there. |
jameskoster
requested review from
a team and
fluiddot
and removed request for
a team
@paaljoachim There's a subtle difference in that the post editor preview will show unpublished changes, so it slightly different to just viewing the post. That may be a reason not to change the wording. |
|
Thanks for the reply, @talldan We could perhaps do a "Preview Post" / "Preview Page" and cut out the "in new tab" part in the drop down of posts and pages. |
|
Similar to the quote in Anne's comment, I was also wondering if this is the right thing to show when editing say 'Archive' or 'Search', but when I tested I realised it's still quite useful and doesn't feel out of place. I imagine it's quite difficult to include a link directly to the relevant part of the website based on the template being edited. I also wondered if the label could be something like 'View homepage'? It also looks like the linting check has failed, this file needs to be formatted - packages/edit-site/src/components/header/index.js ( |
True, that could be simplified. Usually there's some hidden text for screen readers that says 'Opens in a new tab', so it doesn't necessarily have to be visible. That reminds me, that hidden text should be added in this PR too. @jameskoster That would involve using the |
|
It might be a nice enhancement to explore template-specific 'view' links, but I suspect the logic could get a bit gnarly. Something to explore later methinks :) I find 'View Site' and 'View Homepage' to essentially be the same thing, but happy to change if there are strong feelings either way. |
|
"View Site" the meaning I get is that site is really anywhere on the web site. It is better to keep "View Site" as one would be viewing really any template one has active at the time. Be it an author/category/comments/404 or any other template. As one would modify a template in the site editor and just view the frontend of what the template looks like. |
The link does always takes you to the homepage, as discussed above it won't take you to the template being edited. |
I have not yet found that discussion. As from (pre)viewing a post or page. It opens the actual post/page and not the homepage. So that we are used to seeing the current selected post/page on the frontend and transferring this over to the site editor would also very much help. (Example the various call for testings @annezazu usually adds a how to view the current template on the frontend step by adding an URL directly to the template.) If that is not possible or hard to do at present time, then having a step between would be helpful. As in opening the homepage by clicking "View Site". |
For me, the concept of 'viewing a template on the frontend' doesn't really make sense, and can quickly break down. As an example, what would happen if you tried to 'view' a template that isn't actually in use anywhere at the site? The idea of 'viewing' is content-centric, IE you view a post, a page, a category, etc. I'm not sure we should cross those streams. When the site editing experience is also more content-centric (IE once browse mode is implemented) it will make more sense to have contextual view links. |
|
Good point James! That crossed in the back of my mind. I was thinking that for some/many templates these could be viewed on the frontend. Browse mode will be helpful and solve the issue of viewing the template as it is seen on the frontend. |
|
Nice! Very welcome change. Tested it out and works as expected. One question, should it be
|
There was a problem hiding this comment.
This looks good and I think will be a really useful option. It could be a lower case 's' for 'site'. I often forget the rules around this, but I think most of the UI leans towards sentence case.
Some code formatting needs to be done before this can be merged. I think npm run format -- packages/edit-site/src/components/header/index.js is the command that'll sort that out.
|
Since 'site' is referring to a specific website, doesn't that make it a proper noun and therefore suitable to capitalisation? I am not a grammar expert either :D |
|
We should stick to sentence case as we usually do. |
|
Wouldn't nouns over-rule the case convention? 🤔 |
I refreshed my grammar skills, and came to the conclusion that I don't think I'd consider it a proper noun. If the button said the name of the site (e.g. 'View Making WordPress'), then I think it would be capitalised, but 'site' is a common noun for a website and should be lower case in a sentence. I think a similar sentence would be something like "Exit the building" (common noun, so building is lower case) vs "Exit the Empire State Building" (proper noun. so the name of the building is upper case). BTW, there are e2e tests that are failing and need updating because the test still expects the old name of the button (Preview instead of View) in the post editor. Are you happy to update those @jameskoster? If not, let me know and I'd be happy to add a commit to the branch. |
|
@talldan what's the command to update the tests? 🙏 |
It's a manual process of going through the failing tests and updating the selectors used to interact with the preview button. |
|
Hopefully I did it correctly... let's see! 🙈 |
|
Looks spot on! There's a code formatting issue again to resolve in |
|
Noting that this didn't just change the preview button in the site editor, it changed it in all editors, including the post editor |
What?
Add a link from the site editor to view the site.
Why?
Because it is currently not within easy reach.
How?
Added a new
MenuItemtoPreviewOptionsin edit-site/components/header.As a part of this update I've changed 'Preview' to 'View' for a couple of reasons:
Testing Instructions
Screenshots or screencast
view.site.mp4