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

feat(ia-listings): IA for Listings Plugin #3483

Open
wants to merge 13 commits into
base: epic/ia
Choose a base branch
from

Conversation

ronchambers
Copy link
Collaborator

@ronchambers ronchambers commented Oct 17, 2024

All Submissions:

Changes proposed in this Pull Request:

This code completes Asana Task: "Newspack Information Architecture > IA Listings".

Screenshot of Menu:

ia-listings-changes

Screenshot of Dashboard:

ia-listings-dashboard

  • "Cards" are clickable to Listings menu items.
  • When Listings Plugin is inactive, entire section and cards are removed.

How to test the changes in this Pull Request:

  1. Pull branch.
  2. npm run build
  3. Activate Newspack plugin.
  4. Install/activate Newspack Listings Plugin.
  5. Verify screenshots and notes above.
  6. Deactivate Listings plugin, verify dashboard no longer shows Listings section.

Note about Related PR:

The code to move the submenu link for Marketplace above Generic is done in the Listings Plugin. ( SEE RELATED: Automattic/newspack-listings#487 )

If we are unable to approve that PR, then we'll need to approve this PR keeping Generic above Marketplace (this is how Listings Plugin does it already), or we'll need to update this PR with a solution.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@ronchambers ronchambers changed the title feat(ia-listings): initial dev for Listings feat(ia-listings): IA for Listings Plugin Oct 23, 2024
@ronchambers ronchambers marked this pull request as ready for review October 24, 2024 04:43
@ronchambers ronchambers requested a review from a team as a code owner October 24, 2024 04:43
@ronchambers
Copy link
Collaborator Author

@miguelpeixe - please see above. I'll assign you in Asana and reach out in Slack.

Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

It tested well! I have just a minor request inline.

includes/wizards/class-listings-wizard.php Outdated Show resolved Hide resolved
includes/wizards/newspack/class-newspack-dashboard.php Outdated Show resolved Hide resolved
@miguelpeixe
Copy link
Member

@jaredrethman I've pushed some changes to accommodate the recent refactor to the menu order logic. Could you review this one?

@miguelpeixe miguelpeixe self-assigned this Nov 1, 2024
Copy link
Collaborator

@jaredrethman jaredrethman left a comment

Choose a reason for hiding this comment

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

@miguelpeixe thanks for the review, only thing missing is active submenu when add/edit post types i.e:
/wp-admin/post-new.php?post_type=newspack_lst_*
/wp-admin/post.php?post=POST_ID

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants