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] Update Popover Padding Size #7200

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

danieldong51
Copy link
Contributor

@danieldong51 danieldong51 commented Jul 9, 2024

Description

Set popover padding size to small for instances of non-context menu popovers. Did not change popovers that had no padding to begin with.

Screenshots

Area Before (v7Light) After (v7Light) Before (v8Dark) After (v8Dark)
Appearance Appearance v7 Light Before Appearance v7 Light After Appearance v8 Dark Before Appearance v8 Dark After
Discover Field Discover Field v7 Light Before Discover Field v7 Light After Discover Field v8 Dark Before Discover Field v8 Dark After
Help Help v7 Light Before Help v7 Light After Help v8 Dark Before Help v8 Dark After
Syntax Syntax v7 Light Before Syntax v7 Light After Syntax v8 Dark Before Syntax v8 Dark After
Visualize Visualize Fields v7 Light Before Visualize Field v7 Light After Visualize Field v8 Dark Before Visualize Field v8 Dark After

Changelog

  • refactor: [Look&Feel] Update Popover Padding Size

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 github-actions bot added first-time-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Jul 9, 2024
@virajsanghvi
Copy link
Collaborator

Know this is a draft, but one comment - do you know why the padding for "Syntax" look condensed in the after? I'm guessing we're not using the component correctly?

Copy link
Contributor

github-actions bot commented Jul 9, 2024

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "refactor". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@github-actions github-actions bot added failed changeset and removed Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Jul 9, 2024
@danieldong51
Copy link
Contributor Author

Know this is a draft, but one comment - do you know why the padding for "Syntax" look condensed in the after? I'm guessing we're not using the component correctly?

The font size for the syntax menu is larger than the other popovers (16px vs 14px), that could be partly why the margins seem condensed? Here's what the syntax menu looks like with 14px font:

Syntax Font Change

And here's what the popover looks like with the Help Menu with small padding:
Comparison

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.55%. Comparing base (f7ecefb) to head (7b0434c).
Report is 338 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7200      +/-   ##
==========================================
- Coverage   67.56%   67.55%   -0.01%     
==========================================
  Files        3469     3469              
  Lines       68479    68479              
  Branches    11130    11130              
==========================================
- Hits        46266    46264       -2     
- Misses      19511    19512       +1     
- Partials     2702     2703       +1     
Flag Coverage Δ
Linux_1 33.13% <ø> (ø)
Linux_2 55.26% <ø> (ø)
Linux_3 45.30% <ø> (-0.02%) ⬇️
Linux_4 34.71% <ø> (ø)
Windows_1 33.15% <ø> (ø)
Windows_2 55.21% <ø> (ø)
Windows_3 45.32% <ø> (ø)
Windows_4 34.71% <ø> (ø)

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 virajsanghvi added the look & feel Look and Feel Improvements label Jul 10, 2024
@virajsanghvi
Copy link
Collaborator

virajsanghvi commented Jul 10, 2024

LGTM ,but can you get checks passing so I can approve?

For changelog, think you want to install https://github.com/apps/opensearch-changeset-bot in your repo so it automatically gets created here

@virajsanghvi
Copy link
Collaborator

Restarted failed test to see if it passes

@github-actions github-actions bot added Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry and removed failed changeset labels Jul 10, 2024
Copy link
Contributor

❌ 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.

@github-actions github-actions bot added failed changeset and removed Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry failed changeset labels Jul 10, 2024
opensearch-changeset-bot bot added a commit to danieldong51/OpenSearch-Dashboards that referenced this pull request Jul 10, 2024
Signed-off-by: Dan Dong <danieldong51@gmail.com>
@virajsanghvi virajsanghvi merged commit b422791 into opensearch-project:main Jul 11, 2024
71 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-7200-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b422791347c22029f0cf155ff24492d0ea9e3c8d
# Push it to GitHub
git push --set-upstream origin backport/backport-7200-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-7200-to-2.x.

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

Successfully merging this pull request may close these issues.

3 participants