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

PWA-622: Update Venia's button #2496

Merged
merged 38 commits into from
Jun 26, 2020

Conversation

davemacaulay
Copy link
Contributor

Description

Adjust Button styles to match the new design

Related Issue

https://jira.corp.magento.com/browse/PWA-622

Acceptance

  • Verify all buttons are updated throughout the app
  • Ensure there is no broken functionality with the styling change

Verification Stakeholders

@jimbo
@dhaecker
@soumya-ashok
@schensley

Specification

Verification Steps

  1. Navigate around the app following all known user flows
  2. Verify all buttons previously present are present and work
  3. Verify all buttons are positioned and readable

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have updated the documentation accordingly, if necessary.

anthoula and others added 16 commits June 9, 2020 15:31
- copy tokens to `venia-ui`
- move index.css to `venia-ui`
- update global vars and move to tokens
- update custom property name references
- update serif font fallback value
- update @font-face weights in templates
- add serif @font-face in templates
# Conflicts:
#	packages/venia-concept/template.html
#	packages/venia-ui/lib/components/Footer/footer.css
#	packages/venia-ui/templates/default-font-include.mst
- use absolute paths to import `venia-ui` in `venia-concept`
- update existing CSS custom properties to use the new values
- use functional color notation
- Update hover state to new colors
- Fix various buttons throughout site
- Fix estimate shipping info inconsistencies
# Conflicts:
#	packages/venia-ui/lib/tokens.css
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jun 17, 2020

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: PWA-622.

Generated by 🚫 dangerJS against 138623a

@davemacaulay davemacaulay changed the base branch from develop to PWA-621 June 17, 2020 21:12
@devops-pwa-codebuild
Copy link
Collaborator

devops-pwa-codebuild commented Jun 17, 2020

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2496.pwa-venia.com : LH Performance Expected 0.85 Actual 0.51, LH Best Practices Expected 1 Actual 0.92, WPT Cache Expected 90 Actual 83.333333333333
https://pr-2496.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.35, LH Best Practices Expected 1 Actual 0.92
https://pr-2496.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.41, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.92, WPT Cache Expected 65 Actual 50

@davemacaulay davemacaulay added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Jun 18, 2020
@sirugh
Copy link
Contributor

sirugh commented Jun 18, 2020

Just a quick glance, I noticed the following:

  1. Filter/sort buttons on category page seem to have been missed:

Image from Gyazo

  1. Estimate Shipping on Cart page doesn't match other adjustment button style:

Image from Gyazo

  1. Checkout button on Cart page (and other buttons elsewhere) has a weird shadow thing:

Image from Gyazo

  1. Some icons are blue stroke and some are black stroke:

Image from Gyazo

- Update filter and sort buttons
- Make icons the same color in account area
- Fix shipping estimate button
@davemacaulay
Copy link
Contributor Author

davemacaulay commented Jun 18, 2020

@sirugh

Just a quick glance, I noticed the following:

  1. Filter/sort buttons on category page seem to have been missed:

Good catch, spoke with Soumya and updated to black outline with smaller min width.

  1. Estimate Shipping on Cart page doesn't match other adjustment button style:

Another great catch, fixed.

  1. Checkout button on Cart page (and other buttons elsewhere) has a weird shadow thing:

This is intentional, this is the :focus state for buttons and will be displayed in some (notably Chrome) browsers when you click on a button.

  1. Some icons are blue stroke and some are black stroke:

Didn't know account area was a thing, thanks for bringing it to my attention. Worked with Scott & Soumya and decided to turn the black icon blue.

jimbo
jimbo previously approved these changes Jun 24, 2020
sirugh
sirugh previously approved these changes Jun 24, 2020
@davemacaulay davemacaulay dismissed stale reviews from sirugh and jimbo via d95f194 June 24, 2020 21:36
sirugh
sirugh previously approved these changes Jun 25, 2020
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Three times a charm!

jimbo
jimbo previously approved these changes Jun 25, 2020
@dpatil-magento
Copy link
Contributor

@davemacaulay As discussed Shipping Info and Shipping Method Edit overlay buttons needs to be consistent in guest and Auth mode. Rest all looks good.

- Update auth shipping address edit buttons
@davemacaulay davemacaulay dismissed stale reviews from jimbo and sirugh via 8c6e821 June 26, 2020 14:22
disabled: isSaving,
priority: isUpdate ? 'high' : 'normal',
Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt this was an accident originally. Does the style guide call for all proceed/submit buttons to be high priority?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both 'Update' and 'Add' are considered primary actions in the address modal, I modified this logic to the Save and Continue button is the correct normal priority if the authenticated user doesn't have any saved addresses :)

- Update checkout auth button to be secondary style
sirugh
sirugh previously approved these changes Jun 26, 2020
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

....Fourth?

Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

So many buttons.

@dpatil-magento
Copy link
Contributor

QA Approved.
Follow up ticket will handle -

  1. Mobile View >> Filters >> Reset filter does not get disabled after first tap but clears filters (minor)
  2. Mini cart Edit product - Cancel button UI and order of display is stale (missed to report/update)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:venia-concept pkg:venia-styleguide pkg:venia-ui version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants