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

[Look&Feel] Apply missing guidance for visBuilder #7341

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

danieldong51
Copy link
Contributor

Description

Applies look and feel pattern guidance to components in visBuilder.

Screenshot

Scope Before (v7Light) After (v7Light) Before (v8dark) After (v8dark)
Flyover: Update icon Flyover Update v7 Light Before Flyover Update v7 Light Post Flyover Update v8 Dark Before Flyover Update v8 Dark Post
Modal: Semantic header VisBuilder Modal v7 Light Before VisBuilder Modal v7 Light Post VisBuilder Modal v8 Dark Before VisBuilder Modal v8 Dark Post
Field detail: Adding tooltip Tooltip v7 Light Before Tooltip v7 Light Post Tooltip v8 Dark Before Tooltip v8 Dark Post

Changelog

  • refactor: [Look&Feel] Apply guidance for visBuilder

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

opensearch-changeset-bot bot added a commit to danieldong51/OpenSearch-Dashboards that referenced this pull request Jul 21, 2024
Copy link

codecov bot commented Jul 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.75%. Comparing base (46afb96) to head (e8b28de).
Report is 289 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7341      +/-   ##
==========================================
+ Coverage   67.70%   67.75%   +0.05%     
==========================================
  Files        3521     3521              
  Lines       69777    69783       +6     
  Branches    11390    11390              
==========================================
+ Hits        47244    47283      +39     
+ Misses      19732    19691      -41     
- Partials     2801     2809       +8     
Flag Coverage Δ
Linux_1 33.16% <100.00%> (+<0.01%) ⬆️
Linux_2 55.50% <ø> (+0.04%) ⬆️
Linux_3 43.31% <ø> (?)
Linux_4 34.81% <ø> (ø)
Windows_1 33.18% <100.00%> (+<0.01%) ⬆️
Windows_2 55.45% <ø> (+0.04%) ⬆️
Windows_3 43.31% <ø> (+0.05%) ⬆️
Windows_4 34.81% <ø> (ø)

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.

@danieldong51 danieldong51 changed the title [Look&Feel] Apply guidance for visBuilder [Look&Feel] Apply missing guidance for visBuilder Jul 21, 2024
@zhongnansu zhongnansu added enhancement New feature or request backport 2.x v2.16.0 look & feel Look and Feel Improvements labels Jul 21, 2024
@danieldong51 danieldong51 force-pushed the vis_builder branch 2 times, most recently from 99313c6 to b27424f Compare July 22, 2024 18:09
Signed-off-by: Dan Dong <danieldong51@gmail.com>
Copy link
Collaborator

@virajsanghvi virajsanghvi left a comment

Choose a reason for hiding this comment

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

approving as cypress tests look unrelated

do have question on ignoring typing on imports though

Comment on lines +7 to +11
// @ts-ignore
import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { IndexPatternField } from '../../../../../data/common';
// @ts-ignore
import { findTestSubject } from '@elastic/eui/lib/test';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious are these ignored elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at least in that folder they are - field_details.test.tsx and /utils/get_field_details.test.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since the field details component includes my field bucket component, I mostly used the setup found in field_details.test.tsx

@zhongnansu zhongnansu merged commit 55e23a4 into opensearch-project:main Jul 22, 2024
65 of 67 checks passed
@opensearch-trigger-bot
Copy link
Contributor

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-7341-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 55e23a44488089940c6974d38c0d4591b38595d5
# Push it to GitHub
git push --set-upstream origin backport/backport-7341-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-7341-to-2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants