WordPress / wporg-theme-directory

15 stars 6 forks source link

New block: Theme settings for owners to control community/commercial URLs #71

Closed ryelle closed 3 months ago

ryelle commented 3 months ago

This creates a single new block for theme owners to configure settings related to their theme. Currently the only option is the URL for either support (commercial themes) or a development repo (community themes).

Fixes #34, fixes #35

Screenshots

Before

Community Commercial
Image Image

After

Community Commercial Neither
theme-community theme-commercial theme-none

When saving, the form submits to the API and adds a message next to the button. This message is also announced to screen readers. It does not automatically update the URL at the top of the page, if this is confusing we could possibly add a note to reload the page?

Success Error
Screenshot 2024-05-15 at 5 50 55 PM Screenshot 2024-05-15 at 5 51 26 PM

How to test the changes in this Pull Request:

  1. View a community theme either as the theme owner or a side admin/moderator
  2. You should see the "Theme options" section
  3. Update the field
  4. It should only accept URLs (browser input validation)
  5. Reloading the page, the "Contribute to this theme" link should use the new URL
  6. Repeat testing on a commercial theme
jasmussen commented 3 months ago

Vast improvement.

More of a site-wide-styles question, so not a blocker for this PR, but some of the componentry here could be improved:

Screenshot 2024-05-16 at 09 40 24

We were also missing an example for how to best structure label+input+help text, as far as font sizes, gap, paddings, colors. So I quickly extracted one from the recent Learn work, and put it here:

Screenshot 2024-05-16 at 09 45 29

So small paragraph charcoal-4 helptext, long paragraph label and input.

fcoveram commented 3 months ago

Here is an idea that taps into @jasmussen's

Error and success messages in theme url process

The changes are:

I added a new page in the Themes file where this section has both mockups.What do you think @ryelle?

ryelle commented 3 months ago

I don't really want to spend a lot of time on this, so what do you think about leaving the error message where it is in the success position? Right now I use the same element for that, so it would just take more refactoring to put it into the notice and still have it correctly read by screen readers.

(small edit to give context and not just sound lazy 😅 — right now, this section is not used by many people - it's probably only set once by a theme owner, for just a subset of themes, and then left as-is. if we add more fields to this in the future, we can definitely move around the error & make it more helpful)

The rest seems to be just CSS updates, so that's all fine 👍🏻

fcoveram commented 3 months ago

That sounds good @ryelle 🚀

ryelle commented 3 months ago

Screenshot with the design updates:

Screenshot 2024-05-16 at 10 56 45 AM