[discover] optimize rendering language options #8685

kavilla · 2024-10-23T00:02:07Z

Description

Optimize the language selector preventing too many renders and potentially setting of the language triggering an update.

Issues Resolved

n/a

Screenshot

Testing the changes

Changelog

fix: Optimize rendering the Discover language options

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

github-actions · 2024-10-23T00:02:42Z

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

codecov · 2024-10-23T00:16:51Z

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 60.86%. Comparing base (79fed30) to head (42c611b).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../data/public/ui/query_editor/language_selector.tsx	55.55%	7 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8685      +/-   ##
==========================================
- Coverage   60.86%   60.86%   -0.01%     
==========================================
  Files        3793     3793              
  Lines       90447    90456       +9     
  Branches    14203    14204       +1     
==========================================
+ Hits        55053    55057       +4     
- Misses      31906    31910       +4     
- Partials     3488     3489       +1

Flag	Coverage Δ
Linux_1	`29.08% <ø> (ø)`
Linux_2	`56.40% <ø> (ø)`
Linux_3	`37.69% <55.55%> (+<0.01%)`	⬆️
Linux_4	`29.85% <ø> (ø)`
Windows_1	`29.10% <ø> (ø)`
Windows_2	`56.35% <ø> (ø)`
Windows_3	`37.69% <55.55%> (+<0.01%)`	⬆️
Windows_4	`29.85% <ø> (-0.01%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

virajsanghvi · 2024-10-23T00:42:10Z

src/plugins/data/public/ui/query_editor/language_selector.tsx

+      const isAppSupported =
+        !props.appName || language?.editorSupportedAppNames?.includes(props.appName);
+
+      if (!isSupported || isBlocklisted || !isAppSupported) return;


should this be !isBlocklisted?

tbh i added this advanced setting in case host wanted to prevent specific languages from showing but i doubt it will be used in the situation where this was in experimental and a language wasn't working. this should be removed in general.

What is that advanced setting supposed to do?

virajsanghvi · 2024-10-23T00:44:52Z

src/plugins/data/public/ui/query_editor/language_selector.tsx

+
+  const selectedLanguage = useMemo(
+    () => ({
+      label:


does this need to default to something?

like if it can't find the value? it could it might make sense to pull from the advanced setting

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

github-actions · 2024-10-23T05:49:16Z

❌ Create Or Update Content

Something went wrong. Error creating or updating content in repository. Please try again.

ashwin-pc

LGTM

ashwin-pc · 2024-10-23T06:33:58Z

src/plugins/data/public/ui/query_editor/language_selector.tsx

+      const isAppSupported =
+        !props.appName || language?.editorSupportedAppNames?.includes(props.appName);
+
+      if (!isSupported || isBlocklisted || !isAppSupported) return;


What is that advanced setting supposed to do?

ashwin-pc · 2024-10-23T06:37:12Z

src/plugins/data/public/ui/query_editor/language_selector.tsx


-  languageService.setUserQueryLanguage(currentLanguage);


Was this what caused all those issues?

* memoization plus not setting the user query language Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * clean up commented code Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * Changeset file for PR #8685 created/updated * Changeset file for PR #8685 deleted --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 85e0767) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* memoization plus not setting the user query language * clean up commented code * Changeset file for PR #8685 created/updated * Changeset file for PR #8685 deleted --------- (cherry picked from commit 85e0767) Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

github-actions bot added the distinguished-contributor label Oct 23, 2024

kavilla added backport 2.x discover for discover reinvent discover-next v2.18.0 labels Oct 23, 2024

github-actions bot added the failed changeset label Oct 23, 2024

virajsanghvi reviewed Oct 23, 2024

View reviewed changes

memoization plus not setting the user query language

ee57410

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

kavilla force-pushed the kavilla/fix_language_selected_issue branch from 0173254 to ee57410 Compare October 23, 2024 05:05

clean up commented code

5555739

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

kavilla marked this pull request as ready for review October 23, 2024 05:47

kavilla requested review from ananzh, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba and zhyuanqi as code owners October 23, 2024 05:47

kavilla requested review from mengweieric, LDrago27, sejli, joshuali925 and huyaboo as code owners October 23, 2024 05:47

kavilla changed the title ~~[discover] memoization plus not setting the user query language~~ [discover] optimize rendering language options Oct 23, 2024

Changeset file for PR opensearch-project#8685 created/updated

7782216

github-actions bot removed the failed changeset label Oct 23, 2024

github-actions bot added the failed changeset label Oct 23, 2024

Changeset file for PR opensearch-project#8685 deleted

42c611b

ashwin-pc approved these changes Oct 23, 2024

View reviewed changes

sejli approved these changes Oct 23, 2024

View reviewed changes

ashwin-pc merged commit 85e0767 into opensearch-project:main Oct 23, 2024
67 of 68 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 23, 2024

[Backport 2.x] [discover] optimize rendering language options #8688

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[discover] optimize rendering language options #8685

[discover] optimize rendering language options #8685

kavilla commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024

codecov bot commented Oct 23, 2024 •

edited

Loading

virajsanghvi Oct 23, 2024

kavilla Oct 23, 2024

ashwin-pc Oct 23, 2024

virajsanghvi Oct 23, 2024

kavilla Oct 23, 2024

github-actions bot commented Oct 23, 2024

ashwin-pc left a comment

ashwin-pc Oct 23, 2024

ashwin-pc Oct 23, 2024

[discover] optimize rendering language options #8685

[discover] optimize rendering language options #8685

Conversation

kavilla commented Oct 23, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Oct 23, 2024

❌ Empty Changelog Section

codecov bot commented Oct 23, 2024 • edited Loading

Codecov Report

virajsanghvi Oct 23, 2024

Choose a reason for hiding this comment

kavilla Oct 23, 2024

Choose a reason for hiding this comment

ashwin-pc Oct 23, 2024

Choose a reason for hiding this comment

virajsanghvi Oct 23, 2024

Choose a reason for hiding this comment

kavilla Oct 23, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 23, 2024

❌ Create Or Update Content

ashwin-pc left a comment

Choose a reason for hiding this comment

ashwin-pc Oct 23, 2024

Choose a reason for hiding this comment

ashwin-pc Oct 23, 2024

Choose a reason for hiding this comment

kavilla commented Oct 23, 2024 •

edited

Loading

codecov bot commented Oct 23, 2024 •

edited

Loading