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

fix: cookie page guard should only use NgModal on browser #1148

Merged
merged 1 commit into from
May 5, 2022

Conversation

Eisie96
Copy link
Contributor

@Eisie96 Eisie96 commented May 4, 2022

PR Type

[ x ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Current Behavior?

In a ssr+nginx setup the route: /cookies will show the error page of the nginx. The ssr container have the following log:

image

The cookie page guard should manage to open and close a modal to select the desired cookies. It should be clear, that modals are only available on browser and not on server side. (angular-slider/ngx-slider#66)

Issue Number: Closes #

What Is the New Behavior?

The cookie page guard manages the cookie modal only on browser side.

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

Other Information

Reproduce: Go to demo page: https://intershoppwa.azurewebsites.net/cookies

AB#76445

@Eisie96 Eisie96 requested a review from shauke May 4, 2022 13:14
@Eisie96 Eisie96 self-assigned this May 4, 2022
@Eisie96 Eisie96 added this to the 2.4 milestone May 4, 2022
@Eisie96 Eisie96 merged commit 9a26d87 into develop May 5, 2022
@Eisie96 Eisie96 deleted the fix/open-cookie-route-in-ssr branch May 5, 2022 08:00
shauke pushed a commit that referenced this pull request May 5, 2022
* cookie page guard should only use NgModal on browser side
* ssr returns loading page on entering the `/cookie` route
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants