-
Notifications
You must be signed in to change notification settings - Fork 5
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: losing context when changing language in concepts page #642
Fix: losing context when changing language in concepts page #642
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fix seems fine by me, simplicity is always better.
But can you just add a video, I want to see if refresh page doesn't degrade the UX of refreshing only content part.
Great PR description, btw. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing it, it did not work
Enregistrement.de.l.ecran.2024-06-03.a.14.31.33.mov
It's weird that u r in the Animal husbandry and breeding class without having it in the params of your browser URL. |
@syphax-bouazzouni how did you arrive to this class? |
To reproduce:
|
Yes, you are right, I broke the original behavior when I tried to clean the old code of the language selector, now it's working: Enregistrement.de.l.ecran.2024-06-03.a.16.45.47.mov |
I also tested the case: and it's working correclty |
From this video, I see two other issues: |
Thanks for ur remark, then the only solution is to refrech the whole page: Enregistrement.de.l.ecran.2024-06-03.a.17.02.44.movPS. my dev environment is slower in general it's not related to this |
Nah refreshing all the pages, is old school. The old way of refreshing only the frame was better, even if it is more complex. You just need to know why we did lose the context and fix only that. |
I did already spend a whole day to figure out, but there's nothing and the code was so complicated anyway and it need to be simplified |
@Bilelkihal postpone this, will go back to it in the future. |
cd4f7bb
to
1a843c9
Compare
* fix xml download button in summary page, export metadata * adjust the position of the export metadata download button and make the hover cursor pointer for it
…ab (#539) * refactor concepts json code and put it in a stimulus controller * adjust the position of the concepts json link * remove non related code to concepts json PR * remove non related code to contextual json pull request * pass base class URL directly to concepts json stimulus controller * clean concepts json stimulus code * remove undesired code from concepts json button PR * fix typo in agent_search_input_component.html.haml * rename concepts json button stimulus controller * clean concepts json button controller code * clean concepts json button related code --------- Co-authored-by: Syphax bouazzouni <[email protected]> Co-authored-by: Bilel KIHAL <[email protected]> Co-authored-by: @SirineMhedhbi
…ps and categories (#641)
* add cookies action to show and save cookie state in a session * add link button component helper * add cookie modal view * enhance the style of the cookies banner --------- Co-authored-by: Bilel KIHAL <[email protected]>
…tip (#713) * update summary page categories display to show acronym with tooltip * fix submission flow system tests after changing their display * make show_category return the full domain if not a category * update the show category name to use clickable state if not a category and is a link
* make agents search field component results unlimited to only 4 * make agents cherchable by acronym, affiliations, email and home page * Update the agent search input to display the result in this format Orga name (ACRONYM) – ROR:003vg9w96 and for persons Person Name – ORCID:0000-1524-5487-5487 * chagne display mode value type from string to boolean in search input component * remove agents search logic from UI side and limit it to only name, acronym and identifiers * use the endpoint '/search/agents' for the agents search input component
* Hide affiliations when the agent is an organization in the agents form * Change the name of the function affliations? to is_organization? to make it more clear * fix submission flows test after the fix of the PR #713 * fix agent tests after changing the behavior of displaying affiliations --------- Co-authored-by: Syphax <[email protected]>
…type (#721) * remove the restriction for certain submission fields to create only organization type or person type agents * make orcid or ror default input based on agent type in agent form * fix disappeared signup form icons * fix nested agents creation * fix agent form not filling saved identifier values * create an agent identifier input helper * disable cookie banner in development and test mode * fix agent tests after enforcing ROR for organizations --------- Co-authored-by: Syphax <[email protected]>
* Fix propertyId param auto added to summary page URL * remove added PropertyId param to the URL when opening summary page * Fix auto added propertyId to the summary page URL * Fix auto added instanceid to the summary page URL * Fix auto added propertyId to the summary page URL * add back the constraint of making frames loading lazy while env.development * make by default properties section displays the first property * make by default instances section displays the first instance * change the name of the function instances_tree_first_id by search_first_instance_id * add a clarification comment in search_first_instance_id method * move get_property function to properties controller --------- Co-authored-by: Syphax <[email protected]>
78823b0
to
5312cbe
Compare
…' into fix/change-language-lost-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the code to fix the issue of not saving the good current URL
Not working for the initial use case issue: |
7d3dcdc
to
6b90576
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working ✅
d94b7b4
to
d40389d
Compare
…guage-lost-context
Q&A session - 5th September (Anne sophie & Nina) 1-[Langue des catégories du filtre] : [Quand on met la langue du site en francais les sous categories du filtre "Langues naturelles", "niveaux de formalité" restent en anglais] 2- [Selection d'ontologies multilangues] : [Pas de possibilité de faire un regroupement/intersection d'ontologies qui comprennent plusieurs langues] 3-[Langue de définition des classes] : [La definition d'une classe est décrite en anglais alors que la langue du contenu est le francais] 4-[Langue de synonymes des classes] : [Pas de synonmye francais pour certaines classes] 5-[Orthographe de synonymes des classes] : [Pas d'harmonisation de notation des synonymes] 6-[Labélisation nom préféré pour synonyme des classes] : [Pas label de "nom préféré" pour des synonymes] 7-[Classement des noms préférés des classes] : [En fonction des classes selectionnées, la langue préférée qui apparait en premier varie] 8-[Ordre alphabétique et "All language"] : [Le classement des "Classes" nest plus dans l'ordre alphabétique quand la langue du contenu est "All language"] 9-[Changement du nom des classes en chiffre] : [Quand change la langue du contenu, le nom des classes devient des chiffres] 10- [Ordre des classes avec accent] : [Les classes avec des accents ne sont pas classé par ordre alphabétique, se retrouve en fin de liste] 11- [XX] : [XX] |
Related issues:
#557
#617
agroportal/project-management#518
Description
To see the problem:
1- Pick concept 'A' from the tree view.
2- Change language to French for example.
3- Change language to All languages.
4- Choose concept 'B'.
5- Go back to French.
Now, instead of showing concept 'B' in French, it goes back to showing 'A'.
Technical Cause
Though the exact issue remains somewhat elusive, here's how it unfolds: when the language is changed, the select component triggers an event called 'lang_changed'. This event is intercepted to update the URL via the history stimulus controller, and refresh the content frame via the turbo frame controller
action: "lang_changed->history#updateURL lang_changed->turbo-frame#updateFrame"
In attempting to trace the execution flow leading to the anomaly, I encountered an unusual occurrence within the history stimulus controller. While debugging the JavaScript line by line, I observed an abrupt change in window.location.search to an incorrect concept.
Justification of the Technical Choice
I opted to refresh the page each time the language changes, but using Turbo to do it without reloading the whole browser.
Benifits: