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

Preserve current filter state in URL #34 #211

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

jalezi
Copy link
Collaborator

@jalezi jalezi commented Jan 18, 2022

UPDATE #34

This PR has two parts.

First part commits from 722b6ff to 15064e1:
Refactor localization. Package i18next is used only for initialization. We are using hook from react-i18next package.
Also I removed default language while we are using LanguageDetector.

Second part commits from d13917a to eaecec6:
Preserve current filter state in URL.
All routes for languages and doctor type are not dynamic.

before:

    <Route
      path="/:lang/:type/:name/:instId"
      ...
    />

after:

      <Route
        key={`${lang}-dr-page`}
        path={`/${lang}/${type}/:name/:instId`}
        ...
      />

If this PR is not merged, commit 7c22789 can be still merged. Code is dead.

In some components we were using t() direct from "i18next.
replace useParam with i18n.language to get language.
Language is not used as param but still stays in path.
pass boolean prop to styled component as 1 || 0
before:
url: /sl/wrong-dr-type -> no 404 page

after:
url: /sl/wrong-dr-type -> 404 page
accept and search not preserved if not default value ("vsi")
navigate after doctor type has changed.
Using in Filters is causing unnecesary re-renders
@jalezi jalezi marked this pull request as draft January 18, 2022 17:29
@jalezi jalezi requested a review from stefanb January 18, 2022 17:29
@jalezi
Copy link
Collaborator Author

jalezi commented Jan 18, 2022

I have just find a small bug.

@github-actions github-actions bot temporarily deployed to pr-211 January 18, 2022 17:33 Inactive
todo:
filters not updated if user change hash in address bar
@github-actions github-actions bot temporarily deployed to pr-211 January 19, 2022 12:38 Inactive
@github-actions github-actions bot temporarily deployed to pr-211 January 19, 2022 13:11 Inactive
@jalezi jalezi changed the title Preserve current filter state in URL Preserve current filter state in URL #34 Jan 19, 2022
@jalezi jalezi marked this pull request as ready for review January 19, 2022 13:14
'/',
)}|s-${oldHashValues.search}`;

navigate(`./#${hash}`);
Copy link
Member

Choose a reason for hiding this comment

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

It should not push a new entry into the browser history - pressing back button now only changes search parameters in the url, and possibly moving the map view. It should go to the previous page.

Maybe something like:

Suggested change
navigate(`./#${hash}`);
navigate(`./#${hash}`, { replace: true });

? Didn't test that.

@github-actions github-actions bot temporarily deployed to pr-211 January 24, 2022 08:19 Inactive
on map events we don't want to add url to history stack
@stefanb
Copy link
Member

stefanb commented Jan 24, 2022

Is navigate() used also somewhere else? Search box changes are put into browser history, pressing back is just stepping back the search box queries.

@jalezi
Copy link
Collaborator Author

jalezi commented Jan 24, 2022

Is navigate() used also somewhere else? Search box changes are put into browser history, pressing back is just stepping back the search box queries.

yes in Doctors/index.js. I am stuck. Needs different approach. Unfortunately ATM I don't have time.

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