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

fix: set translated text to link field #15451

Merged
merged 84 commits into from May 16, 2022
Merged

Conversation

hrwX
Copy link
Contributor

@hrwX hrwX commented Dec 26, 2021

  • Fixes Issue/15324
  • When selecting a value for Link Field, the English text was set as link field value.
  • This PR aims to fix it by setting the translated text as link field value on selection.

Todo:

  • Show translated text in the select dropdown
  • Set translated value to Link field on select
  • Show original value when link field is in focus
  • Add option to toggle this behaviour

Behaviour

  • Link field loses focus: show the translated name.
  • Link field is focused
    • If old value is present
      • If options are one of [Role, DocType]: show the translated name
      • Else: show the name (untranslated) to enable search in untranslated values
    • Else: show what the user typed (untranslated) to enable search in untranslated values
  • Value is selected: link field loses focus

Demo

Link to UOM in a custom Item DocType

The UOM names are in english, so the search needs to happen in english. When possible, the translation is displayed.

Before

Bildschirmaufnahme.2022-03-02.um.18.26.01.mov

After

Bildschirmaufnahme.2022-03-02.um.17.58.34.mov

Link to DocType in Customize Form

The DocType names get translated before being searched. This is a preexisting hack in the framework for DocType and Role. In this case, we can search in the translations.

Before

Bildschirmaufnahme.2022-03-02.um.18.22.22.mov

After

Bildschirmaufnahme.2022-03-02.um.18.07.02.mov

https://docs.erpnext.com/docs/v13/user/manual/en/customize-erpnext/customize-form/edit?wiki_page_patch=fdafee2715

@barredterra barredterra self-requested a review December 27, 2021 11:01
@barredterra
Copy link
Collaborator

barredterra commented Dec 28, 2021

Bildschirmfoto 2021-12-28 um 15 43 11

When selecting the link, I can search only in german, but the results will show only in english. This is still a bit confusing.

Once selected the value is translated, which is great.

On other link fields it works well, maybe this is just affecting Customize Form.

@barredterra
Copy link
Collaborator

Bildschirmaufnahme.2021-12-28.um.15.46.44.mov

@hrwX
Copy link
Contributor Author

hrwX commented Dec 28, 2021

@barredterra

  • The customize form doc_type was excluded from translation.
  • Fixed. It'll be translated now.
  • Screenshot from 2021-12-29 03-00-33

@hrwX hrwX marked this pull request as ready for review December 28, 2021 21:34
@hrwX hrwX requested a review from a team as a code owner December 28, 2021 21:34
@hrwX hrwX requested review from surajshetty3416 and removed request for a team December 28, 2021 21:34
@stale
Copy link

stale bot commented Jan 5, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Jan 5, 2022
Copy link
Collaborator

@barredterra barredterra left a comment

Choose a reason for hiding this comment

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

@hrwX this is working great! A big improvement of localized user experience 🚀

Copy link
Collaborator

@barredterra barredterra left a comment

Choose a reason for hiding this comment

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

Looks like cypress tests for the link field need to be updated as well

@codecov
Copy link

codecov bot commented Jan 15, 2022

Codecov Report

Merging #15451 (4d581ab) into develop (8a61c74) will increase coverage by 0.03%.
The diff coverage is 78.57%.

@@             Coverage Diff             @@
##           develop   #15451      +/-   ##
===========================================
+ Coverage    58.18%   58.22%   +0.03%     
===========================================
  Files          768      768              
  Lines        68783    68811      +28     
  Branches      5977     5985       +8     
===========================================
+ Hits         40020    40062      +42     
+ Misses       25134    25133       -1     
+ Partials      3629     3616      -13     
Flag Coverage Δ
server 63.76% <100.00%> (+0.04%) ⬆️
ui-tests 48.68% <75.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@hrwX hrwX force-pushed the translate_link_text branch 2 times, most recently from 79b46be to b8e44ae Compare January 16, 2022 19:46
@gavindsouza gavindsouza assigned barredterra and unassigned hrwX May 9, 2022
@hrwX
Copy link
Contributor Author

hrwX commented May 9, 2022

@hrwX one scenario does not seem to work yet:

  1. Take a doctype "A" that is not in translated_search_doctypes hook
  2. Activate "Translate Link Fields" for "A"
  3. Set your language to something other than english
  4. Open a record of docytpe "B" that has a link to "A"
  5. Set the link once
  6. Edit the link

Observed: typing in English, I see translated results (I have to guess what english words lead to what translated results). After selection, I see the translated value. On edit, I keep seeing and editing the translated value. Therefore I get no more results (searching translation in untranslated values).

Expected: typing in English, I see English results (or both English and translated). After selection, I see the translated value. On edit, I see and edit the untranslated value.

Alternatively, we can make translated_search_doctypes and "Translate Link Fields" the same thing, eliminating above scenario.

Screencast from 05-05-2022 09_04_19 AM

@fproldan
Copy link

Hi, @barredterra @hrwX this would be backported to v13?

@barredterra
Copy link
Collaborator

@fproldan that's the goal. But I can't promise we'll make it, as tricky as this PR is. :D

@barredterra
Copy link
Collaborator

Functionality LGTM now

@barredterra barredterra added the add-docs New feature should be have an entry in documentation to increase the discoverability label May 10, 2022
@barredterra
Copy link
Collaborator

@hrwX please make sure the tests are working and add docs for this feature and it's delimination to / interaction with translated_search_doctypes hooks.

@hrwX
Copy link
Contributor Author

hrwX commented May 11, 2022

@hrwX hrwX removed the add-docs New feature should be have an entry in documentation to increase the discoverability label May 11, 2022
@barredterra
Copy link
Collaborator

Test with DocType Country: "Translate Link Fields" checked in doctype definition and "Country" in translated_search_doctypes hook. Searches in translations, displays translation, saves original value:

country.mov

Test with DocType Module Def: "Translate Link Fields" not checked in doctype definition and "Module Def" not in translated_search_doctypes hook. Searches in original values, displays translation, saves original value:

Bildschirmaufnahme.2022-05-16.um.14.56.07.mov

@mergify mergify bot merged commit 70409a3 into frappe:develop May 16, 2022
fproldan pushed a commit to fproldan/frappe that referenced this pull request May 16, 2022
- Fixes [Issue/15324](frappe#15324)
- When selecting a value for Link Field, the English text was set as link field value.
- This PR aims to fix it by setting the translated text as link field value on selection.

Todo:

- [x] Show translated text in the select dropdown
- [x] Set translated value to Link field on select
- [x] Show original value when link field is in focus
- [x] Add option to toggle this behaviour

- Link field loses focus: show the translated name.
- Link field is focused
    - If old value is present
        - If options **are** one of [Role, DocType]: show the translated name
        - Else: show the name (untranslated) to enable search in untranslated values
    - Else: show what the user typed (untranslated) to enable search in untranslated values
- Value is selected: link field loses focus

The UOM names are in english, so the search needs to happen in english. When possible, the translation is displayed.

https://user-images.githubusercontent.com/14891507/156415248-e5e80d05-53dc-4ca8-89c7-998986ff6e99.mov

https://user-images.githubusercontent.com/14891507/156410386-a874430c-f340-43ed-9c3a-92e8d4d50fc9.mov

The DocType names get translated before being searched. This is a preexisting hack in the framework for DocType and Role. In this case, we can search in the translations.

https://user-images.githubusercontent.com/14891507/156414648-8e505f8c-9dee-4358-8182-3b358c28bb62.mov

https://user-images.githubusercontent.com/14891507/156411881-c4ca22e1-1397-4e13-9768-5e16b72f8d6d.mov

https://docs.erpnext.com/docs/v13/user/manual/en/customize-erpnext/customize-form/edit?wiki_page_patch=fdafee2715
@barredterra
Copy link
Collaborator

@fproldan you can find the backport to version-13 above.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Translate link fields bug: translations in link fields
9 participants