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

Master backmerge into Develop #3011

Merged
merged 26 commits into from
Feb 23, 2021
Merged

Master backmerge into Develop #3011

merged 26 commits into from
Feb 23, 2021

Conversation

revanth0212
Copy link
Contributor

@revanth0212 revanth0212 commented Feb 11, 2021

❌ DON'T SQUASH MERGE THIS PR ❌

Description

Backmerging master branch into develop branch.

Related Issue

Closes PWA-1333

Verification Stakeholders

@jimbo @dpatil-magento

Verification Steps

A smoke test on the deployed app. Nothing should break, everything should fine.

Screenshots / Screen Captures (if appropriate)

Checklist

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

tjwiebell and others added 21 commits December 23, 2020 10:48
* Initial work.

* Lil better with loading states.

* Using debounce to fetch order number.

* Re arranging useEffects.

* Updated CSS.

* Order search useEffect logic change.

* Using onCompleted instead.

* Final touchups.

* Minor.

* Addressed UX review.

* Using submit button to search for orders.

* Minor.

* Minor.

* Added orderHistoryPage tests.

* Using fragments.

* Using fuzzy search.

* Added useOrderHistory tests.

* Using single query for order and orders data.

* Fetch all orders if search text is empty.

* Allowing button submit even if the search field is empty to fetch all orders.

* Test update.
* Working PoC of moving form context to just wrap search inputs

* Add simple pagination logic

* - Fix rendering of reset button
- Add Load More button and wire up to orders query

* Increase page size to UX requirement and remove unnecessary currentPage argument

* Add page info label to top of page

* Cover changes with tests

* - Use token for font size
- Make callback optional and document
* Moves getPageSize query to talon default operation

Signed-off-by: sirugh <rugh@adobe.com>

* Only show search page error if there is no data.

Signed-off-by: sirugh <rugh@adobe.com>

* Only render errors when error AND no data

Signed-off-by: sirugh <rugh@adobe.com>

* Update tests

Signed-off-by: sirugh <rugh@adobe.com>

* Make sure we only do the page reset if there is no data

Signed-off-by: sirugh <rugh@adobe.com>

* Test page setting effect

Signed-off-by: sirugh <rugh@adobe.com>

Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
* Clear out old 8.0.0 content

* Add link references

* Add page 1 PR entries

* Add page 2 entries

* Add page 3 entries

* Add page 4 and 5 entries

* Add details to highlights

* Update compatibility table

* Update highlight and known issue

* Update Top Community Contributors

* Update content based on feedback

* Fix capitalization

* Add missing entries

Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
* Add known issue about watcher running out of memory

* Add link

* Update note about watcher duration
* Remove lodash from create-pwa

* Fix bug

* Extend fix to venia-concept

* Using map instead of set.

* Pass sample backends into create script

* Fixup

* Restore sample backend export

* Fix unit test

Co-authored-by: Revanth Kumar <revanth0212@gmail.com>
* Create release notes for 9.0.1

* Fix link heading

* Add another PR entry

Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
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.

Didn't catch the conflict resolutions snuck into that merge commit, did have some minor feedback that needs addressed.

Comment on lines 29 to 30
if (loading) return fullPageLoadingIndicator;
else if (error) {
Copy link
Contributor

@tjwiebell tjwiebell Feb 22, 2021

Choose a reason for hiding this comment

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

Theses two gates are unreachable now. I think we should have just accepted develop in this resolution, eg.

if (loading && !product) return fullPageLoadingIndicator;
if (error && !product) return <ErrorView />;
if (!product) { // OOS Message }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right @tjwiebell. @sirugh what do you think should be the result if there is an error?

In develop we are rendering

<ErrorView />

And in master we are rendering

<div>
     <FormattedMessage
            id={'product.errorFetch'}
            defaultMessage={'Data Fetch Error'}
       />
</div>

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should use what is in develop, as @tjwiebell suggested.

tjwiebell
tjwiebell previously approved these changes Feb 22, 2021
@dpatil-magento
Copy link
Contributor

@revanth0212 looks good, just unhide these links now #2918

@revanth0212
Copy link
Contributor Author

@revanth0212 looks good, just unhide these links now #2918

@dpatil-magento done 👍

@dpatil-magento
Copy link
Contributor

QA Approved.

@dpatil-magento dpatil-magento merged commit 39ceaa3 into develop Feb 23, 2021
@dpatil-magento dpatil-magento deleted the revanth/master_9.0 branch February 23, 2021 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:create-pwa pkg:extensions pkg:pagebuilder pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:venia-concept pkg:venia-ui Progress: done 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.

8 participants