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.

Sign up for GitHub

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

Jump to bottom

Fix path routing #500

Merged
merged 8 commits into from
Jul 31, 2019
Merged

Fix path routing #500

merged 8 commits into from
Jul 31, 2019

Conversation

y-kkamil
Copy link

@y-kkamil y-kkamil commented Apr 26, 2019 •
edited by jesusreal
Loading

Description

Changes proposed in this pull request:

  • Fixes the problem with reloaded micro frontend view when using path routing

Related issue(s)

https://github.com/kyma-project/console/issues/798, which includes the steps to reproduce the issue.

@y-kkamil y-kkamil closed this Apr 26, 2019
@y-kkamil y-kkamil reopened this Apr 26, 2019
@y-kkamil
Copy link
Author

Fixes #491

@y-kkamil y-kkamil marked this pull request as ready for review April 26, 2019 14:04
@y-kkamil
Copy link
Author

Checked for angular/vue/react routers. Please test for any use case you can think of. ;)
Unfortunately, https://github.com/kyma-project/console/issues/798 still persists.

@jesusreal jesusreal self-assigned this Apr 29, 2019
@maxmarkus maxmarkus added area/luigi bug Something isn't working labels Apr 29, 2019
@jesusreal jesusreal removed their assignment Apr 30, 2019
@maxmarkus maxmarkus added this to the Sprint_Swinka_13 milestone May 3, 2019
@y-kkamil y-kkamil mentioned this pull request May 15, 2019
Closed
@hardl hardl removed the area/luigi label May 15, 2019
@jesusreal jesusreal added the WIP Work in progress label May 22, 2019
@jesusreal
Copy link
Contributor

jesusreal commented May 22, 2019 •
edited
Loading

This issue is still blocked until https://github.com/kyma-project/console/issues/798 is solved.

@hardl hardl modified the milestones: Sprint_0, Sprint_1 Jun 12, 2019
@jesusreal jesusreal removed this from the Sprint_1 milestone Jun 17, 2019
@jesusreal jesusreal added this to the Backlog milestone Jun 17, 2019
@CLAassistant
Copy link

CLAassistant commented Jul 3, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@y-kkamil
Copy link
Author

y-kkamil commented Jul 24, 2019 •
edited
Loading

react-router uses a library called history internally. https://github.com/ReactTraining/history/blob/master/modules/DOMUtils.js#L53 --> this is the line that was causing problems for console. Adding state to PopStateEvent makes the expression evaluate to false, which in turn lets the router take care of route change.

@jesusreal jesusreal removed the WIP Work in progress label Jul 25, 2019
@jesusreal jesusreal mentioned this pull request Jul 25, 2019
Closed
@jesusreal jesusreal self-assigned this Jul 26, 2019
jesusreal
jesusreal approved these changes Jul 26, 2019
View reviewed changes
Copy link
Contributor

@jesusreal jesusreal left a comment

Choose a reason for hiding this comment

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

Tested it with Kyma console, Catalog node, navigating between the Services and the Addons tab. It works 👍

@marynaKhromova marynaKhromova self-assigned this Jul 29, 2019
marynaKhromova
marynaKhromova approved these changes Jul 29, 2019
View reviewed changes
Copy link
Contributor

@marynaKhromova marynaKhromova left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@jesusreal jesusreal merged commit c879efd into SAP:master Jul 31, 2019
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
@jesusreal
Fix broken path routing for react apps (SAP#500)
4f4ca2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jesusreal jesusreal jesusreal approved these changes

@marynaKhromova marynaKhromova marynaKhromova approved these changes

@antiheld antiheld Awaiting requested review from antiheld

@dariadomagala-sap dariadomagala-sap Awaiting requested review from dariadomagala-sap

@hardl hardl Awaiting requested review from hardl hardl is a code owner

@JohannesDoberer JohannesDoberer Awaiting requested review from JohannesDoberer JohannesDoberer is a code owner

@kwiatekus kwiatekus Awaiting requested review from kwiatekus

@maxmarkus maxmarkus Awaiting requested review from maxmarkus

@parostatkiem-zz parostatkiem-zz Awaiting requested review from parostatkiem-zz

@pekura pekura Awaiting requested review from pekura

Assignees

@jesusreal jesusreal

@marynaKhromova marynaKhromova

Labels
bug Something isn't working
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.
6 participants
@y-kkamil @jesusreal @CLAassistant @marynaKhromova @maxmarkus @hardl