[Discover Next] Fixes Discover styles #7546

ashwin-pc · 2024-07-28T14:30:16Z

Description

Adds the correct styling to the discover 2.0 page. This combines 3 changes:

Add styling for canvas for query enhancement #7537 (@abbyhu2000)
Add styling for discover filter selector when query enhancement is on #7536 (@abbyhu2000)
Styling for the query bar

These styles are only applied when the query enhancements toggle is on.

Issues Resolved

Screenshot

Screen.Recording.2024-07-28.at.7.27.21.AM.mov

With toggle off:

Testing the changes

Changelog

fix: Fixes Discover next styling

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

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

codecov · 2024-07-28T14:44:34Z

Codecov Report

Attention: Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.

Project coverage is 63.65%. Comparing base (7287699) to head (ecfa4b8).
Report is 247 commits behind head on main.

Files with missing lines	Patch %	Lines
...ation/components/sidebar/discover_field_search.tsx	54.54%	3 Missing and 2 partials ⚠️
.../data/public/ui/query_editor/language_selector.tsx	66.66%	2 Missing ⚠️
.../data_explorer/public/components/app_container.tsx	33.33%	0 Missing and 2 partials ⚠️
.../public/ui/dataset_navigator/dataset_navigator.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7546      +/-   ##
==========================================
- Coverage   63.65%   63.65%   -0.01%     
==========================================
  Files        3629     3629              
  Lines       79509    79519      +10     
  Branches    12599    12602       +3     
==========================================
+ Hits        50612    50616       +4     
- Misses      25832    25835       +3     
- Partials     3065     3068       +3

Flag	Coverage Δ
Linux_1	`30.76% <66.66%> (ø)`
Linux_2	`55.58% <ø> (ø)`
Linux_3	`40.23% <56.52%> (-0.01%)`	⬇️
Linux_4	`31.57% <66.66%> (ø)`
Windows_1	`30.78% <66.66%> (ø)`
Windows_2	`55.53% <ø> (ø)`
Windows_3	`40.24% <56.52%> (+<0.01%)`	⬆️
Windows_4	`31.57% <66.66%> (ø)`

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.

AMoo-Miki · 2024-07-29T07:57:13Z

src/plugins/data/public/ui/dataset_navigator/dataset_navigator.tsx

+  const datasetTitle =
+    selectedDataSetState &&
+    selectedDataSetState?.dataSourceRef &&
+    selectedDataSetState?.dataSourceRef.name
+      ? `${selectedDataSetState.dataSourceRef?.name}::${selectedDataSetState?.title}`
+      : selectedDataSetState?.title;


Suggested change

const datasetTitle =

selectedDataSetState &&

selectedDataSetState?.dataSourceRef &&

selectedDataSetState?.dataSourceRef.name

? `${selectedDataSetState.dataSourceRef?.name}::${selectedDataSetState?.title}`

: selectedDataSetState?.title;

const datasetTitle =

selectedDataSetState?.dataSourceRef?.name

? `${selectedDataSetState.dataSourceRef.name}::${selectedDataSetState.title}`

: selectedDataSetState?.title;

sejli

LGTM

sejli · 2024-07-29T08:40:56Z

src/plugins/data/public/ui/query_editor/editors/default_editor/index.tsx

Comments in this file could be removed

Paul is adding his auto complete stuff here. I wanted to leave the markers in place for his style changes

ruanyl

Approved with minor comments, and by the way, I tried to run this at my local but getting this loading screen which kind of confusing, if something didn't work, would be better to show an error message.

ruanyl · 2024-07-29T09:49:25Z

src/plugins/data/public/ui/query_editor/editors/shared.tsx

+  Body: BodyComponent<TBody>;
+}
+
+interface EditorInstance<TCollapsed, TExpanded, TBody> {


Nit: TCollapsed, TExpanded, TBody these are defined but seems not used.

Oh good catch, I dont need both interfaces, just the first one.

ruanyl · 2024-07-29T09:51:20Z

src/plugins/data/public/ui/query_editor/editors/shared.tsx

+  onChange,
+  editorDidMount,
+}) => (
+  <div className="osdQuerEditor__singleLine">


Nit: typo

Suggested change

<div className="osdQuerEditor__singleLine">

<div className="osdQueryEditor__singleLine">

ruanyl · 2024-07-29T10:00:47Z

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

+          <div className="osdQueryEditor__body">{languageEditor.Body()}</div>
+        )}
+
+        {/*  <EuiFlexGroup gutterSize="xs" direction="column">


Nit: why not just cleanup unused code?

This refactor focussed on styling changes, but could not validate all the features the query bar intiially had such as Query assist and auto complete since they were in a partially reverted state. Preserved the previous code to make reincorporating them correctly easier. SHould be cleaned up in a followup PR

ruanyl · 2024-07-29T10:02:34Z

src/plugins/data/public/ui/query_editor/types.ts

+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export interface LanguageEditor<T = {}> {


This is defined, but never used

ruanyl · 2024-07-29T10:09:35Z

src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx

@@ -75,13 +77,19 @@ export interface Props {
   * types for the type filter
   */
  types: string[];
+  isEnhancementsEnabledOverride: boolean;


Nit: would be nice to have a comment to describe isEnhancementsEnabledOverride

ruanyl · 2024-07-29T10:11:33Z

src/plugins/discover/public/application/view_components/canvas/discover_chart_container.tsx

  const { indexPattern, savedSearch } = useDiscoverContext();
+  const isEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);


Nit: just for readability

Suggested change

const isEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);

const isQueryEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);

ashwin-pc · 2024-07-29T17:04:51Z

CI group 4 is a known bug with a fix thats incoming. Al other tests pass

opensearch-trigger-bot · 2024-07-29T17:06:35Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7546-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2d8c743266223dd1420c85133c56b50e5d96be46
# Push it to GitHub
git push --set-upstream origin backport/backport-7546-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7546-to-2.x.

opensearch-trigger-bot · 2024-07-29T17:06:36Z

The backport to 2.16 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.16 2.16
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.16
# Create a new branch
git switch --create backport/backport-7546-to-2.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2d8c743266223dd1420c85133c56b50e5d96be46
# Push it to GitHub
git push --set-upstream origin backport/backport-7546-to-2.16
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.16

Then, create a pull request where the base branch is 2.16 and the compare/head branch is backport/backport-7546-to-2.16.

ashwin-pc · 2024-07-29T18:14:19Z

@ruanyl thanks. The default state is a known bug that existed in main already. @kavilla is working on a fix for that

* fixes the query bar UI Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * incorporates Abby's page styling Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * fixes tests Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * Changeset file for PR #7546 created/updated --------- Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 2d8c743) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

opensearch-trigger-bot · 2024-07-29T20:43:48Z

The backport to 2.16 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.16 2.16
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.16
# Create a new branch
git switch --create backport/backport-7546-to-2.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2d8c743266223dd1420c85133c56b50e5d96be46
# Push it to GitHub
git push --set-upstream origin backport/backport-7546-to-2.16
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.16

Then, create a pull request where the base branch is 2.16 and the compare/head branch is backport/backport-7546-to-2.16.

* fixes the query bar UI * incorporates Abby's page styling * fixes tests * Changeset file for PR #7546 created/updated --------- (cherry picked from commit 2d8c743) Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.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>

After the following PRs: #7492, #7546, #7540 this commit added skip(1) back to dataset manager observable: fef6156, we need to revert changes done in fix(query assist): update reading data source id from dataset manager #7464 (comment) revert dataset manager observable usage in query assist to support skip(1) revert dataset manager tests [Discover Next] Fixes Discover styles #7546 removed query editor header div, this PR adds it back to enable query editor extensions Signed-off-by: Joshua Li <joshuali925@gmail.com>

After the following PRs: opensearch-project#7492, opensearch-project#7546, opensearch-project#7540 this commit added skip(1) back to dataset manager observable: fef6156, we need to revert changes done in fix(query assist): update reading data source id from dataset manager opensearch-project#7464 (comment) revert dataset manager observable usage in query assist to support skip(1) revert dataset manager tests [Discover Next] Fixes Discover styles opensearch-project#7546 removed query editor header div, this PR adds it back to enable query editor extensions Signed-off-by: Joshua Li <joshuali925@gmail.com>

* [Discover 2.0] Updating fetch functions to include local cluster (#7542) * Update datasources fetch function to include local cluster * Check for duplicates when fetching external datasources (in the case local cluster is added as a datasource) * Clean up types in DataSetNavigator so items are displayed properly --------- Signed-off-by: Sean Li <lnse@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> * [discover-next][bug] add max height to dataset navigator and use memoization (#7540) * add max heigh. use memoization Signed-off-by: Kawika Avilla <kavilla414@gmail.com> almost working pretty nicely Signed-off-by: Kawika Avilla <kavilla414@gmail.com> a little bit better Signed-off-by: Kawika Avilla <kavilla414@gmail.com> its ok Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * update mock Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * update another mock Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * fix mock for extension Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * rebase fixes Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * update script Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * fix initial load Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * Fix query assist for query editor (#7552) After the following PRs: #7492, #7546, #7540 this commit added skip(1) back to dataset manager observable: fef6156, we need to revert changes done in fix(query assist): update reading data source id from dataset manager #7464 (comment) revert dataset manager observable usage in query assist to support skip(1) revert dataset manager tests [Discover Next] Fixes Discover styles #7546 removed query editor header div, this PR adds it back to enable query editor extensions Signed-off-by: Joshua Li <joshuali925@gmail.com> * [Discover next] Fixes dataset navigator menu styling & search error toast (#7566) Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * [Discover 2.0] Loading fix for databases (#7567) * add back in useeffect for loading databases Signed-off-by: Sean Li <lnse@amazon.com> * Changeset file for PR #7567 created/updated --------- Signed-off-by: Sean Li <lnse@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --------- Signed-off-by: Sean Li <lnse@amazon.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> Co-authored-by: Sean Li <lnse@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: Joshua Li <joshuali925@gmail.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>

* fixes the query bar UI Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * incorporates Abby's page styling Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * fixes tests Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> * Changeset file for PR opensearch-project#7546 created/updated --------- Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

After the following PRs: opensearch-project#7492, opensearch-project#7546, opensearch-project#7540 this commit added skip(1) back to dataset manager observable: fef6156, we need to revert changes done in fix(query assist): update reading data source id from dataset manager opensearch-project#7464 (comment) revert dataset manager observable usage in query assist to support skip(1) revert dataset manager tests [Discover Next] Fixes Discover styles opensearch-project#7546 removed query editor header div, this PR adds it back to enable query editor extensions Signed-off-by: Joshua Li <joshuali925@gmail.com>

ashwin-pc added 2 commits July 28, 2024 05:27

fixes the query bar UI

ebd0b11

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

incorporates Abby's page styling

81f1ee1

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

ashwin-pc added backport 2.x v2.16.0 discover-next backport 2.16 labels Jul 28, 2024

ashwin-pc requested review from ananzh, kavilla, AMoo-Miki, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba, zhyuanqi, mengweieric, LDrago27, virajsanghvi and sejli as code owners July 28, 2024 14:30

github-actions bot added the distinguished-contributor label Jul 28, 2024

opensearch-changeset-bot bot added a commit to ashwin-pc/OpenSearch-Dashboards that referenced this pull request Jul 28, 2024

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

e8a5b71

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

ecfa4b8

ashwin-pc force-pushed the fix/query-styles branch from e8a5b71 to ecfa4b8 Compare July 29, 2024 06:47

AMoo-Miki reviewed Jul 29, 2024

View reviewed changes

sejli approved these changes Jul 29, 2024

View reviewed changes

ashwin-pc changed the title ~~Fix/query styles~~ [Discover Next] Fixes Discover styles Jul 29, 2024

ruanyl approved these changes Jul 29, 2024

View reviewed changes

ashwin-pc mentioned this pull request Jul 29, 2024

Add styling for canvas for query enhancement #7537

Closed

7 tasks

ashwin-pc merged commit 2d8c743 into opensearch-project:main Jul 29, 2024
65 of 68 checks passed

opensearch-trigger-bot bot added the failed backport label Jul 29, 2024

joshuali925 mentioned this pull request Jul 29, 2024

Fix query assist after #7492, #7546, #7540 #7552

Merged

7 tasks

abbyhu2000 mentioned this pull request Jul 29, 2024

Add styling for discover filter selector when query enhancement is on #7536

Closed

7 tasks

LDrago27 added backport 2.x and removed backport 2.x labels Jul 29, 2024

opensearch-trigger-bot bot mentioned this pull request Jul 29, 2024

[Backport 2.x] [Discover Next] Fixes Discover styles #7554

Merged

LDrago27 added backport 2.16 and removed backport 2.16 labels Jul 29, 2024

ashwin-pc removed the failed backport label Sep 12, 2024

AMoo-Miki added the in-next label Sep 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover Next] Fixes Discover styles #7546

[Discover Next] Fixes Discover styles #7546

ashwin-pc commented Jul 28, 2024

codecov bot commented Jul 28, 2024 •

edited

Loading

AMoo-Miki Jul 29, 2024

sejli left a comment

sejli Jul 29, 2024

ashwin-pc Jul 29, 2024

ruanyl left a comment

ruanyl Jul 29, 2024

ashwin-pc Jul 29, 2024

ruanyl Jul 29, 2024

ruanyl Jul 29, 2024

ashwin-pc Jul 29, 2024

ruanyl Jul 29, 2024

ruanyl Jul 29, 2024

ruanyl Jul 29, 2024

ashwin-pc commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

ashwin-pc commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

	<div className="osdQuerEditor__singleLine">
	<div className="osdQueryEditor__singleLine">

		const { indexPattern, savedSearch } = useDiscoverContext();
		const isEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);

	const isEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);
	const isQueryEnhancementsEnabled = uiSettings.get(QUERY_ENHANCEMENT_ENABLED_SETTING);

[Discover Next] Fixes Discover styles #7546

[Discover Next] Fixes Discover styles #7546

Conversation

ashwin-pc commented Jul 28, 2024

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Jul 28, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

sejli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanyl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwin-pc commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

ashwin-pc commented Jul 29, 2024

opensearch-trigger-bot bot commented Jul 29, 2024

codecov bot commented Jul 28, 2024 •

edited

Loading