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.

Sign up for GitHub

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

Highlight the anchor row in surrounding doc view #7025

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Highlight the anchor row in surrounding doc view #7025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4a9074
highlight the anchor row
abbyhu2000 Jun 13, 2024
7b666db
Changeset file for PR #7025 created/updated
opensearch-changeset-bot[bot] Jun 13, 2024
fa81e81
Merge branch 'main' into highlight_anchor_row
abbyhu2000 Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/7025.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Highlight the anchor row in surrounding doc view ([#7025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7025))
2 changes: 1 addition & 1 deletion src/plugins/discover/public/application/components/default_discover_table/table_row.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const TableRowUI = ({
]);

const tableRow = (
<tr key={row._id}>
<tr key={row._id} className={row.isAnchor ? 'osdDocTable__row--highlight' : ''}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a test for this or is it covered in some other way?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think default discover table is covered by any unit tests. src/plugins/discover/public/application/components/default_discover_table

Copy link
Member

Choose a reason for hiding this comment

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

Should we add a ftr test? cuz we want to gradually build up the test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we want to add unit tests and functional tests for default discover table overall. Created an issue to track it here: #7048

<td data-test-subj="docTableExpandToggleColumn" className="osdDocTableCell__toggleDetails">
<EuiButtonIcon
color="text"
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/discover/public/application/doc_views/doc_views_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ import { ComponentType } from 'react';
import { SearchResponse } from 'elasticsearch';
import { IndexPattern } from '../../../../data/public';

export type OpenSearchSearchHit<T = unknown> = SearchResponse<T>['hits']['hits'][number];
export type OpenSearchSearchHit<T = unknown> = SearchResponse<T>['hits']['hits'][number] & {
Copy link
Member

Choose a reason for hiding this comment

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

nit: the name OpenSearchSearchHit is confusing for me because it seems like OpenSearch is returning back to us if it is an anchor or not. Can we create another alias for this to be more descriptive that shims to the hit response if it isAnchor or not based on the context?

that would help me as well not worry if there is potential issues changing the type (even though it is just an optional field). if this type is actually only used within Discover in context then perhaps the name can be reconsidered since OpenSearchSearchHit implies to me it has more purposes than just for table rows.

isAnchor?: boolean;
};

export interface FieldMapping {
filterable?: boolean;
Expand Down
Loading