Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Home Screen: Add Store Management (Quick Links) card #4350

Merged
merged 21 commits into from
May 19, 2020

Conversation

mattsherman
Copy link
Contributor

@mattsherman mattsherman commented May 12, 2020

Fixes #4082

This PR adds the new "Store Management" (Quick Links) card to the home screen.

Accessibility

Screenshots

ScreenCapture at Sat May 16 06:44:50 EDT 2020

Detailed test instructions

  • Enable debugging of Tracks in your browser console:
localStorage.setItem( 'debug', 'wc-admin:*' );
  • View the new home screen.
  • Verify that the "Store Management" card is shown.
  • Click items and verify that they navigate to the correct locations (from Quick Links (Manage your store) card  #4082)
    • When the user clicks "Market my store" he will be redirected to WooCommerce - Marketing
    • When the user clicks "Add products" he will be redirected to WooCommerce - Products
    • When the user clicks "Personalize my store" he will be redirected to Appearance - Customize
    • When the user clicks "Shipping settings" he will be redirected to WooCommerce - Settings - Shipping
    • When the user clicks "Tax settings" he will be redirected to WooCommerce - Settings - Tax
    • When the user clicks "Payment settings" he will be redirected to WooCommerce - Settings - Payments
    • When the user clicks "Edit store details" he will be redirected to WooCommerce - Settings - General
    • When the user clicks "View my store" he will be redirected to the site frontend
  • Verify that wcadmin_home_quick_links_click Tracks events are recorded, with the task_name custom prop set appropriately, every time an item in the list is clicked.

Unit tests

npm test -- -i client/quick-links/test/index.js

Known issues

  • Icons do not match design. (/cc @jameskoster Where can I find the correct icons to use?)
  • Card always shows on home screen. It should only appear when the "Set up your store and start selling" task list card has been completed or manually dismissed. This will be addressed after the task list card has been merged (Add existing TaskList component to new home screen #4378)
  • Card has incorrect ARIA roles (#4387). This should be addressed separately, as it is a problem with the underlying <List> component.
  • Support link always goes to premium support. This should be addressed separately, as it is a core problem with support links in general (Support link in the plugin list confusing and/or incorrect woocommerce#25442), and a solution should be implemented to make sure that support links are consistent throughout the product.
  • Storybook does not show styles correctly for <QuickLinks> component

Changelog Note:

Enhancement: New "Store Management" quick links card on WooCommerce home screen.

@mattsherman mattsherman marked this pull request as draft May 12, 2020 11:57
@mattsherman mattsherman changed the title Homepage: Add Quick Links card Homepage: Add Store Management (Quick Links) card May 12, 2020
@mattsherman mattsherman added the focus: home screen Issues around the new home screen feature label May 12, 2020
@mattsherman mattsherman self-assigned this May 12, 2020
@mattsherman mattsherman mentioned this pull request May 12, 2020
4 tasks
@mattsherman mattsherman changed the title Homepage: Add Store Management (Quick Links) card Home Screen: Add Store Management (Quick Links) card May 16, 2020
@mattsherman mattsherman marked this pull request as ready for review May 16, 2020 10:51
@psealock
Copy link
Collaborator

I went the items to test and just noting it down here while I continue the review.

  • Verify that the "Store Management" card is shown.
  • When the user clicks "Market my store" he will be redirected to WooCommerce - Marketing
  • When the user clicks "Add products" he will be redirected to WooCommerce - Products
  • When the user clicks "Personalize my store" he will be redirected to Appearance - Customize
  • When the user clicks "Shipping settings" he will be redirected to WooCommerce - Settings - Shipping
  • When the user clicks "Tax settings" he will be redirected to WooCommerce - Settings - Tax
  • When the user clicks "Payment settings" he will be redirected to WooCommerce - Settings - Payments
  • When the user clicks "Edit store details" he will be redirected to WooCommerce - Settings - General
  • When the user clicks "View my store" he will be redirected to the site frontend
  • Verify that wcadmin_home_quick_links_click Tracks events are recorded, with the task_name custom prop set appropriately, every time an item in the list is clicked.

Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

This is looking great @mattsherman and testing well ! Just some comments about a few relatively minor things.

client/homepage/style.scss Show resolved Hide resolved
client/homepage/style.scss Show resolved Hide resolved
client/homepage/style.scss Outdated Show resolved Hide resolved
client/homepage/style.scss Outdated Show resolved Hide resolved
client/quick-links/index.js Outdated Show resolved Hide resolved
client/quick-links/index.js Show resolved Hide resolved
client/quick-links/index.js Outdated Show resolved Hide resolved
client/quick-links/index.js Outdated Show resolved Hide resolved
client/quick-links/style.scss Outdated Show resolved Hide resolved
client/quick-links/style.scss Outdated Show resolved Hide resolved
@psealock psealock added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed [Status] Needs Review labels May 18, 2020
@jameskoster jameskoster self-requested a review May 18, 2020 09:30
Copy link
Member

@jameskoster jameskoster left a comment

Choose a reason for hiding this comment

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

@mattsherman the icons are a part of the WordPress core icons package and should be accessible via the Icon component. :)

@mattsherman
Copy link
Contributor Author

@jameskoster All of the icons used in the design (https://www.figma.com/file/Z2PiNnkhjrUIiR7IrfJRnU/Home-Screen?node-id=970%3A0) are not available in the Icon component. And, those that do differ in style from what you used.

It would be very helpful if you could provide me with the name of each icon to use, so that I can ensure that things are implemented as desired. Right now, I'm using a combination of Icon (and therefore dashicon) and gridicons icons to pick things that look close to what the design showed.

@mattsherman
Copy link
Contributor Author

@psealock Thanks for the review and feedback! I've addressed your feedback in the latest commits.

If you feel the PR is ready to be approved outside of the icon changes, I can always take care of those in a followup PR.

@mattsherman mattsherman added [Status] Needs Review and removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. labels May 18, 2020
Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

This is looking fantastic and thanks for the changes. LGTM and good to follow up with icons separately.

@jameskoster
Copy link
Member

All of the icons used in the design are not available in the Icon component. And, those that do differ in style from what you used.

Hm, is that the core Icon component? They should be there because I added them.

At least, I added the ones that I thought were missing, maybe some others need updating from their Dashicon counterparts 🤔

Here's the explicit list of icons to use;

It looks like the Brush and Pencil icons will need to be updated to match the visual design of the others, but we'll handle that in core :)

Sorry for the confusion here.

@mattsherman
Copy link
Contributor Author

All of the icons used in the design are not available in the Icon component. And, those that do differ in style from what you used.

Hm, is that the core Icon component? They should be there because I added them.

At least, I added the ones that I thought were missing, maybe some others need updating from their Dashicon counterparts 🤔

Here's the explicit list of icons to use;
[...]
Sorry for the confusion here.

@jameskoster Thanks for the links to the icons to use. I think the confusion stemmed from my use of Icon from @wordpress/components instead of from @wordpress/icons. I've created a new issue to address updating the icons (#4396), so that I can merge this PR. I'll get a PR up for that and make sure to request a review from you on it so we can ensure that the correct icons are used!

@mattsherman mattsherman dismissed jameskoster’s stale review May 19, 2020 13:14

Updating of icons will be addressed in #4396

@mattsherman mattsherman merged commit d5a187d into master May 19, 2020
@mattsherman mattsherman deleted the add/homepage-quick-links branch May 19, 2020 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: home screen Issues around the new home screen feature tool: monorepo infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quick Links (Manage your store) card
3 participants