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

Improvements to Site Logo block #30406

Closed
12 tasks done
mtias opened this issue Mar 31, 2021 · 28 comments
Closed
12 tasks done

Improvements to Site Logo block #30406

mtias opened this issue Mar 31, 2021 · 28 comments
Labels
[Block] Site Logo Affects the Site Logo Block Needs Design Needs design efforts.

Comments

@mtias
Copy link
Member

mtias commented Mar 31, 2021

The site logo is currently quite rough. This is a list of improvements to make.

image

@jasmussen
Copy link
Contributor

Nice one, I'm going to take a stab at a few of these. Though note, when the site logo is used in the right context — in my case the site editor, I'm seeing the alignments control correctly show up:

Screenshot 2021-04-06 at 11 53 08

I'm not sure why they don't show up when used in the post editor, @scruffian do you recall?

@jasmussen
Copy link
Contributor

CC: @ajlende, I know you have your plate full, but wanted to point to the cropping task in case it's an easy one.

@JustinyAhin
Copy link
Member

JustinyAhin commented Apr 6, 2021

Just jumping on those
I'd like to tackle the width issue

@jasmussen
Copy link
Contributor

@JustinyAhin I've fixed the width issue in a PR I'm currently making 🙈

@JustinyAhin
Copy link
Member

Oh good to know @jasmussen .
Moving to my usual triage then 😀

@jasmussen
Copy link
Contributor

The Site Logo shows up even if my theme has not declared support using add_theme_support. This is probably good, but it did surface a few issues:

  • Once I define a site logo and publish it, I cannot unset it just from the block.

I ended up having to switch to a theme that did support custom logos, then removing it in the customizer.

@jameskoster
Copy link
Contributor

Allow using the site logo for the site icon from the block inspector — this is an important flow.

I think it should be possible for users to do this elsewhere as well. I shouldn't have to be displaying my Site Logo in order to set the Site Icon. In fact I ran in to this exact use case when helping a friend with their site over the weekend. They wanted to set a favicon, but also wanted a plain text "logo" in their site header.

Likewise I shouldn't have to go in and edit a template to update my logo. This should be possible of course, but not the only way.

I would vote for adding these to the WP settings alongside the Site Title and Tagline. The blocks (and potentially other UIs in the editor) can then manipulate these settings ad hoc.

@jasmussen
Copy link
Contributor

I took a stab at a few of the items on this list in #30526. Specifically the width, the placeholder, and the description.

@jameskoster
Copy link
Contributor

If a logo hasn't already been set, I think it might be fun to programatically generate a logo upon insertion. This would fast-forward the user closer to where they want to be, and eliminate the need to the extra UI (placeholder) since they can simply use the already-familiar image tools to replace the auto-generated placeholder.

logo

@mtias
Copy link
Member Author

mtias commented Apr 16, 2021

Description updated in #30909

@carolinan
Copy link
Contributor

The alignments are only available if the logo is placed inside another block. It is a feature or side effect of the new way to handle layout/width settings #30568

@hedgefield
Copy link

Thanks for the PR Joen, it looks good! I do like the idea of a makeshift logo being generated, kinda like the icons in the plugin directory. I wonder how intuitive it is though for someone just expecting to get a box to upload a logo into. It might be a nice feature to have as a choice: upload your own or let WordPress generate something.

Is the needs design tag still on this issue for the 'allow site logo for site icon' flow?

@mtias
Copy link
Member Author

mtias commented Apr 29, 2021

The thing that still needs design work is how to present the site icon option.

@carolinan
Copy link
Contributor

I pictured that option as a simple toggle in the block settings sidebar. Did you want it in the block toolbar?

@hedgefield
Copy link

Me too, something like a "Use site logo as icon" checkbox.

@mtias
Copy link
Member Author

mtias commented Apr 29, 2021

Definitely in settings, but we should properly explain what it is and we should consider the case when no site icon is set and you upload a logo, should we automatically set icon in such cases or ask users if they also want to use it there?

@creativecoder
Copy link
Contributor

Related to these improvements, this seems like a good time to get the logo option name and data type aligned with existing conventions: #31511

@jffng
Copy link
Contributor

jffng commented May 5, 2021

Mentioning a small bug fix that will help for removing some logic from theme templates: #31513

@hedgefield
Copy link

hedgefield commented May 6, 2021

Thinking about this some more I'm wondering where you would be able to set a site icon in the first place? I know where to find it in the old Customizer, but in FSE there's no block for it because it's not a visible element on the page, and I can't really find it under Templates either...

Ah I see that there's a separate issue for that #29126. I'll comment there too.

It does sound logical to have some kind of shortcut in the logo block for the site icon, but then I'd rather that is a link to the Site Icon setting itself, otherwise you're controlling two things from two places - like, would the "use logo as site icon" option on the logo block overrule what you set in the site icon option? Complex.

For the explanation, I think we can use the same one that exists in the customizer now:

Screen Shot 2021-05-06 at 13 40 13

@mtias
Copy link
Member Author

mtias commented May 6, 2021

@hedgefield we should at least account for the flow of uploading a site logo when no site icon is set; there should be a way (or maybe it's automatic) to also use that image for the site icon.

@ajlende
Copy link
Contributor

ajlende commented May 7, 2021

Adding cropping in #31607 and updated the description to reflect that.

@carolinan
Copy link
Contributor

There is also a problem with the resizing. #29217

@hedgefield
Copy link

How's this?

icon

@jameskoster
Copy link
Contributor

In #28341 (comment) I shared a concept that could allow folks to manipulate homepage settings in the midst of an editor session, via modal.

I wonder if there is potential to connect some dots here? IE clicking the "Site icon settings" link in @hedgefield's concept could potentially open the same "Site settings" modal, at a section that contains the site icon configuration options:

site.icon.mp4

@hedgefield
Copy link

That sounds like a proper good idea 👍

@youknowriad
Copy link
Contributor

While trying to integrate this block into Core we had feedback about its storage. This is tracked separately here #32065 and is important to land in 5.8. For the remaining improvements tracked on this issue, we should consider them case by case as we're passed the feature freeze date.

@ramonjd
Copy link
Member

ramonjd commented Nov 5, 2021

Hello!

I think @stacimc's PR Site Logo: Add option to set site icon from Site Logo block #35892 is working well. I think I'm ready to approve, but it'd be good to get some confidence check 👀 from folks, or maybe @creativecoder (whose name I plucked from #32065).

Thank you!

@annezazu
Copy link
Contributor

annezazu commented Jan 4, 2022

Just noting that #35892 got merged and have checked off that item above. This seems to be the final issue here so wondering if we can close? :) cc @priethor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Site Logo Affects the Site Logo Block Needs Design Needs design efforts.
Projects
None yet
Development

No branches or pull requests