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

Wrap user-facing strings for internalization #334

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

nung22
Copy link
Contributor

@nung22 nung22 commented Oct 17, 2023

Description

Updates user-facing strings in the Search Relevance plugin to be consistent with OpenSearch Dashboards guidelines. Strings are now wrapped and compatible with the default message extraction tool.

Issues Resolved

Closes #330

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@nung22
Copy link
Contributor Author

nung22 commented Oct 17, 2023

I have only updated user-facing strings in flyout.tsx as of now. If the format of the FormattedMessage components (specifically naming convention for the id attribute) is satisfactory, I can proceed with the rest of the files.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #334 (a77bfa7) into main (966aed5) will decrease coverage by 4.64%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #334      +/-   ##
==========================================
- Coverage   87.08%   82.44%   -4.64%     
==========================================
  Files          16       32      +16     
  Lines         209      393     +184     
  Branches       43       77      +34     
==========================================
+ Hits          182      324     +142     
- Misses         26       49      +23     
- Partials        1       20      +19     
Flag Coverage Δ
dashboards-search-relevance 82.44% <0.00%> (-4.64%) ⬇️

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

Files Coverage Δ
public/components/common/header.tsx 100.00% <ø> (ø)
...re/search_result/result_components/result_grid.tsx 84.61% <ø> (-6.30%) ⬇️
...are/search_result/search_components/search_bar.tsx 66.66% <ø> (-8.34%) ⬇️
public/components/common/flyout.tsx 50.00% <0.00%> (-33.34%) ⬇️

... and 23 files with indirect coverage changes

bar. When you enter <strong>Search</strong>, the queries are sent to the search engine
using the <EuiCode>GET</EuiCode> HTTP method and the <EuiCode>_search</EuiCode>{' '}
endpoint.
<FormattedMessage
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to wrap this entire in a single FormattedMessage block? May need to use brackets {} for the sub-components for formatting, like strong.

Copy link
Contributor Author

@nung22 nung22 Oct 23, 2023

Choose a reason for hiding this comment

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

Thanks for the suggestion! I made changes to consolidate some of the FormattedMessage blocks.

@sejli
Copy link
Member

sejli commented Oct 17, 2023

Looks good so far! Could we see if we can wrap the entire string in FormattedMessage as per my comment? May be a little more readable. String IDs look good too, @joshuarrrr could you verify if this is a good starting place for wrapping in FormattedMessage?

Thanks for picking this up, this is great!

commit 7e11c9f
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 17:27:21 2023 -0700

    Resolve id and unique key errors

    Signed-off-by: Nicholas Ung <[email protected]>

commit 58f5ac7
Merge: e9b6894 fde4421
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 17:12:18 2023 -0700

    Merge branch 'refactor/issue-330' of https://github.com/nung22/dashboards-search-relevance into refactor/issue-330

commit e9b6894
Merge: 13f9246 e92057a
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 17:08:49 2023 -0700

    Wrap all repo tests with I18nProvider

    Signed-off-by: Nicholas Ung <[email protected]>

commit fde4421
Merge: 13f9246 e92057a
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 17:08:49 2023 -0700

    Merge branch 'refactor/issue-330' of https://github.com/nung22/dashboards-search-relevance into refactor/issue-330

commit 13f9246
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 16:59:05 2023 -0700

    Wrap all repo tests with I18nProvider

commit e92057a
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 16:59:05 2023 -0700

    Wrap test with I18nProvider

    Signed-off-by: Nicholas Ung <[email protected]>

commit 2b9c0ea
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 16:40:25 2023 -0700

    Update duplicate ids in flyout.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit 7d14e3c
Merge: 6869944 bbb27f4
Author: Nicholas Ung <[email protected]>
Date:   Thu Oct 26 10:52:55 2023 -0700

    Merge branch 'refactor/issue-330' of https://github.com/nung22/dashboards-search-relevance into refactor/issue-330

commit 6869944
Author: Nicholas Ung <[email protected]>
Date:   Wed Oct 25 14:35:32 2023 -0700

    Refactor code for user-wrapped strings in header.tsx and flyout.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit b69ff41
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 15:06:05 2023 -0700

    Wrap user-facing strings in header.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit fe99311
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 14:09:06 2023 -0700

    Update flyout.tsx snapshot

    Signed-off-by: Nicholas Ung <[email protected]>

commit 7a2a97a
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 14:03:12 2023 -0700

    Update FormattedMessages blocks in flyout.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit c58a10b
Author: Nicholas Ung <[email protected]>
Date:   Tue Oct 17 10:11:15 2023 -0700

    Wrap user-facing strings in flyout.tsx for internalization

    Signed-off-by: Nicholas Ung <[email protected]>

commit bbb27f4
Merge: 38162ca 77db981
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 17:00:01 2023 -0700

    Merge branch 'main' of https://github.com/nung22/dashboards-search-relevance into refactor/issue-330

commit 38162ca
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 15:06:05 2023 -0700

    Wrap user-facing strings in header.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit a7ce8f6
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 14:09:06 2023 -0700

    Update flyout.tsx snapshot

    Signed-off-by: Nicholas Ung <[email protected]>

commit c1cd4fa
Author: Nicholas Ung <[email protected]>
Date:   Mon Oct 23 14:03:12 2023 -0700

    Update FormattedMessages blocks in flyout.tsx

    Signed-off-by: Nicholas Ung <[email protected]>

commit 611546a
Author: Nicholas Ung <[email protected]>
Date:   Tue Oct 17 10:11:15 2023 -0700

    Wrap user-facing strings in flyout.tsx for internalization

    Signed-off-by: Nicholas Ung <[email protected]>

Signed-off-by: Nicholas Ung <[email protected]>
@nung22 nung22 force-pushed the refactor/issue-330 branch from 7e11c9f to a231956 Compare October 27, 2023 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

[Refactoring] User-Facing Strings Should Be Wrapped
3 participants