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 withoutSync() history behaviour #1720

Merged
merged 8 commits into from
Nov 30, 2020
Merged

Fix withoutSync() history behaviour #1720

merged 8 commits into from
Nov 30, 2020

Conversation

asmeeee
Copy link
Contributor

@asmeeee asmeeee commented Nov 24, 2020

Description

Changes proposed in this pull request:

  • .withoutSync() should use history replaceState instead of pushState

Automatically formatted by Prettier via Husky. Relevant line: 286.

Related issue(s)
Fixes #1719

@CLAassistant
Copy link

CLAassistant commented Nov 24, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@asmeeee
Copy link
Contributor Author

asmeeee commented Nov 24, 2020

Based on my understanding, the following change should be enough to close #1719, but please correct me if I'm wrong.

Routing.navigateTo(path);
Routing.navigateTo(path, isNavigationSyncEnabled);

@asmeeee
Copy link
Contributor Author

asmeeee commented Nov 24, 2020

@hardl Hey! Is there a reason for Travis CI pipeline not to run on this PR?

@hardl hardl added this to the Sprint 14 milestone Nov 24, 2020
@UlianaMunich UlianaMunich added the bug Something isn't working label Nov 24, 2020
@hardl
Copy link
Contributor

hardl commented Nov 24, 2020

@hardl Hey! Is there a reason for Travis CI pipeline not to run on this PR?

was just in the queue ;)

@maxmarkus maxmarkus self-assigned this Nov 26, 2020
maxmarkus
maxmarkus approved these changes Nov 26, 2020
View reviewed changes
Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

LGTM 👍 and since there is a separate ticket for the according e2e test, I accept this one.

@JohannesDoberer JohannesDoberer self-assigned this Nov 30, 2020
@JohannesDoberer JohannesDoberer merged commit c84c82f into SAP:master Nov 30, 2020
@asmeeee asmeeee deleted the fix-withoutsync-history-behaviour branch November 30, 2020 15:18
JohannesDoberer added a commit that referenced this pull request Dec 2, 2020
@JohannesDoberer
Merge branch 'master' into feature-user-settings
b2f4026 
* master:
  update luigi in fiddle
  Change Search using Luigi.globalSearch() API (#1690) (#1734)
  Fix `withoutSync()` history behaviour (#1720)
  Update index.scss
  Bump highlight.js in /core/examples/luigi-example-vue (#1723)
JohannesDoberer added a commit to JohannesDoberer/luigi that referenced this pull request Dec 2, 2020
@JohannesDoberer
Merge branch 'feature-user-settings' into dialog-box-for-user-settings
64ef0df 
* feature-user-settings:
  update luigi in fiddle
  Change Search using Luigi.globalSearch() API (SAP#1690) (SAP#1734)
  Fix `withoutSync()` history behaviour (SAP#1720)
  Update index.scss
  Bump highlight.js in /core/examples/luigi-example-vue (SAP#1723)
@JohannesDoberer JohannesDoberer mentioned this pull request Dec 4, 2020
Merged
@JohannesDoberer JohannesDoberer mentioned this pull request Dec 4, 2020
Closed
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
@asmeeee
Fix withoutSync() history behaviour (SAP#1720)
a2be094
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxmarkus maxmarkus maxmarkus approved these changes

@JohannesDoberer JohannesDoberer JohannesDoberer approved these changes

@alexandra-simeonova alexandra-simeonova Awaiting requested review from alexandra-simeonova

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

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

@stanleychh stanleychh Awaiting requested review from stanleychh

@UlianaMunich UlianaMunich Awaiting requested review from UlianaMunich

Assignees

@maxmarkus maxmarkus

@JohannesDoberer JohannesDoberer

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

Successfully merging this pull request may close these issues.

withoutsync should use history replaceState instead of pushstate
6 participants
@asmeeee @CLAassistant @hardl @maxmarkus @JohannesDoberer @UlianaMunich