GEN-670 - fix: change ssn warning dialog glitch #2794
Merged
+103
−87
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
guilhermespopolin
force-pushed
the
07-17-GEN-670_modal_glitch
branch
from
4440b48
to
acf4df8
Compare
guilhermespopolin
changed the title
guilhermespopolin
force-pushed
the
07-17-GEN-670_modal_glitch
branch
from
acf4df8
to
44bd4bd
Compare
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
alebedev
requested changes
Jul 18, 2023
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
robinandeer
reviewed
Jul 18, 2023
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
guilhermespopolin
force-pushed
the
07-17-GEN-670_modal_glitch
branch
from
44bd4bd
to
8222891
Compare
| datadogLogs.logger.info('Cleared shopSession to change SSN in price calculator') | ||
|
|
||
| const url = new URL(window.location.href) | ||
| if (!url.searchParams.has(OPEN_PRICE_CALCULATOR_QUERY_PARAM)) { |
There was a problem hiding this comment.
Solves one issue we're currently having where that query param might be added several times.
alebedev
approved these changes
Jul 18, 2023
robinandeer
approved these changes
Jul 18, 2023
apps/store/src/components/PriceCalculator/PriceCalculatorAccordion.tsx
Outdated
Show resolved
Hide resolved
| url.searchParams.append(OPEN_PRICE_CALCULATOR_QUERY_PARAM, '1') | ||
| } | ||
|
|
||
| await router.replace(url) |
There was a problem hiding this comment.
should we just do a hard page reload here or are we sure this works?
guilhermespopolin
force-pushed
the
07-17-GEN-670_modal_glitch
branch
from
8222891
to
018f844
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
ChangeSsnWarningDialogJustify why they are needed
As it can be seen in the video below, the glitch happens because
ChangeSsnWarningDialogget's shown for a brief moment when we load information for an authenticated user. In those situations we need to display BankId Dialog butChangeSsnWarningDialogalso get's show because:ChangeSsnWarningDialogappearance is based on the presence ofshopsSession.customer?.ssn.SsnSeSectionwill be re-rendered but this timeshopsSession.customerwill be present - causing the appearance ofChangeSsnWarningDialog.;I think what we want in this case is a more imperative solution where we get to decide when to show
ChangeSsnWarningDialog- when "Edit" button get's clicked and we already have a loaded customer.Screen.Recording.2023-07-18.at.10.15.47.mov