search

kiwicom / margarita

[not actively maintained] Mobile and Web application implementing Kiwi.com Tequila API
https://margarita.kiwi.com/
MIT License
209 stars 42 forks source link

Fix/persist params update search #834

Closed KarinaDavtyan closed 5 years ago

KarinaDavtyan commented 5 years ago

Closes #778

netlify[bot] commented 5 years ago

Storybook from packages/universal-components

Built with commit 88ee32a6d755c31b169298f924180212911ec24c

https://deploy-preview-834--kiwicom-universal-components-old.netlify.com

FilipMessa commented 5 years ago

Just last thing when I removed the location for the Destination placekicker, it set up to Anywhere, but after the refresh, the last chosen Location set back.

But it can be handled in another PR.

KarinaDavtyan commented 5 years ago

Great job, 👍 I like that, I just wrote a few notes, and maybe will be great just try to find a way how to avoid this, long pass on Submit through props. Maby we could handle the onSubimt (changeURL) in the SearchContext. Because you can access to router inside the /margarita/apps/web/pages/_app.js.

But your solution is OK this is just an Idea and also I don't know if it will be work fine.

I ll try, it should be yet improved for sure.

FilipMessa commented 5 years ago

any update?

KarinaDavtyan commented 5 years ago

any update?

@FilipMessa Sorry, will fix it now. Was on holidays

FilipMessa commented 5 years ago

@FilipMessa Sorry, will fix it now. Was on holidays

@KarinaDavtyan Hello Karina, sure no problem, thanks

FilipMessa commented 5 years ago

I found Issue on mobile when I changed the flight type I was redirected immediately on the result page. Jun-17-2019 11-31-37

KarinaDavtyan commented 5 years ago

I found Issue on mobile when I changed the flight type I was redirected immediately on the result page.

Hmm interesting, will look into that. Thanks for spotting it!

KarinaDavtyan commented 5 years ago

I found Issue on mobile when I changed the flight type I was redirected immediately on the result page.

@FilipMessa should be behaving properly now. But I ve discovered other issue, when it goes to /results with oneWay param, it still shows as if it was return. Would it be in scope of this PR to fix it? (the same behavior is on master)

ezgif com-video-to-gif

FilipMessa commented 5 years ago

@KarinaDavtyan Thanks I will check, and related bug what you found it should be in another PR.

FilipMessa commented 5 years ago

@KarinaDavtyan LGTM now, BTW will you take care about this bug or?

KarinaDavtyan commented 5 years ago

@KarinaDavtyan LGTM now, BTW will you take care about this bug or?

@FilipMessa Yes I will