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

Single Site: Update site meta list #144

Merged
merged 2 commits into from
Aug 9, 2023
Merged

Single Site: Update site meta list #144

merged 2 commits into from
Aug 9, 2023

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Aug 8, 2023

See #132 — The site information section has been redesigned to list out each meta value and taxonomy separately. It also updates the layout of the single site page to better match the design, though with the spacing conversation ongoing I didn't aim for perfection.

The taxonomy values are links (so you can click to navigate to all sites tagged "dining"), while the meta field items are not links (there was no plan to allow filtering by these).

Screenshots

Some examples of different sites with various meta filled in or not.

Screen Shot 2023-08-08 at 14 50 00 Screen Shot 2023-08-08 at 14 50 16 Screen Shot 2023-08-08 at 14 50 33

To test

  1. Check out the branch and build it
  2. View a site
  3. It should list the post meta, empty values are not shown
  4. View other sites, or update fields in the editor
  5. Site meta should display as expected

@ryelle ryelle added the [Component] Theme Templates, patterns, CSS label Aug 8, 2023
@ryelle ryelle added this to the MVP milestone Aug 8, 2023
@ryelle ryelle self-assigned this Aug 8, 2023
'key' => 'country',
),
array(
'label' => __( 'Theme', 'wporg' ),
'key' => 'theme',
'label' => __( 'Category', 'wporg' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we limit the category to 1?
Screenshot 2023-08-09 at 9 46 11 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the issue that you don't want to see Featured? We can omit that category, but I don't think this value needs to be limited to one.

white-space: nowrap;
padding-right: 1rem;
}
li {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noting, in the designs, the links are text-decoration: none;.

<!-- /wp:heading -->

<!-- wp:wporg/site-meta-list /-->
</div>
<!-- /wp:group -->

<!-- wp:paragraph {"style":{"spacing":{"margin":{"top":"var:preset|spacing|30"}}}} -->
<p style="margin-top:var(--wp--preset--spacing--30)"><a href="<?php echo esc_url( home_url( '/tags/' ) ); ?>"><?php _e( 'View all tags', 'wporg' ); ?></a></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think margin-top:var(--wp--preset--spacing--20) feels more natural. Not a strong opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's 30px in the design, and also like I mentioned, the spacing conversation is ongoing so I didn't aim for perfection.

Copy link
Collaborator

@StevenDufresne StevenDufresne left a comment

Choose a reason for hiding this comment

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

Looks good. A couple minor comments.

display: grid;
grid-template-columns: 1fr 1fr;
}
.wp-block-wporg-site-meta-list {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It gets a bit tight at 895px.

Screenshot 2023-08-09 at 9 51 31 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can iterate on the responsive style of the page as part of #132

@ryelle
Copy link
Contributor Author

ryelle commented Aug 9, 2023

I'm going to merge this so I can get it on the staging site, but we can keep iterating on it.

@ryelle ryelle merged commit 3f7e7e0 into main Aug 9, 2023
1 check passed
@ryelle ryelle deleted the update/site-meta-list branch August 9, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme Templates, patterns, CSS
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants