Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[bug]: Incorrect Order Id Displayed - Replacing functionality #1249

Merged
merged 7 commits into from
May 23, 2019

Conversation

sirugh
Copy link
Contributor

@sirugh sirugh commented May 16, 2019

Description

  1. Refactor Receipt to be a functional component.
  2. Remove order ID text from display.
  3. Add "View Order Details" button for authed users. Currently no-op as there is no order details page.
  4. Update text/style to match mock.

Related Issue

Closes #816.

Verification Steps

  1. As an authed user, check out and view the receipt. Should match mocks.
  2. As a guest, check out and view the receipt. Should match mocks.

How Have YOU Tested this?

Verification steps above!

Screenshots / Screen Captures (if appropriate):

Proposed Labels for Change Type/Package

  • major (e.g x.0.0 - a breaking change)
  • minor (e.g 0.x.0 - a backwards compatible addition)
  • patch (e.g 0.0.x - a bug fix)

Checklist:

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

@vercel
Copy link

vercel bot commented May 16, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://venia-git-bugfix-receiptcontent.magento-research1.now.sh

Copy link
Contributor

@tjwiebell tjwiebell 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 and functions as expected. Suggested some minor cleanup we can do of legacy code that will clean this component up a little.

Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

Added some clarification to one unresolved comment, everything else looks good.

@sirugh
Copy link
Contributor Author

sirugh commented May 21, 2019

Updated with last bit of feedback. This is how they look:

Image from Gyazo

tjwiebell
tjwiebell previously approved these changes May 21, 2019
Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

🍻

@tjwiebell tjwiebell removed their assignment May 21, 2019
@dpatil-magento
Copy link
Contributor

@sirugh "...by creating and account" should it be "...by creating an account" , typo?

@vercel vercel bot temporarily deployed to staging May 21, 2019 22:10 Inactive
@sirugh
Copy link
Contributor Author

sirugh commented May 21, 2019

@dpatil-magento fixed

@dpatil-magento
Copy link
Contributor

QA Pass. Waiting for #20 to get merged.

@supernova-at supernova-at merged commit afcbb76 into develop May 23, 2019
@supernova-at supernova-at deleted the bugfix/receipt_content branch May 23, 2019 21:33
@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label May 24, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Update Content of Checkout Receipt
4 participants