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

Condence context menus #7057

Merged
merged 12 commits into from
Jul 16, 2024

Conversation

AMoo-Miki
Copy link
Collaborator

@AMoo-Miki AMoo-Miki commented Jun 19, 2024

Hold for @kgcreative and Viraj's signoff.

condenced

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: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Signed-off-by: Miki <miki@amazon.com>
Copy link
Contributor

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

Copy link
Contributor

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.44%. Comparing base (a4aa682) to head (3525031).
Report is 37 commits behind head on main.

Files Patch % Lines
...gins/share/public/components/url_panel_content.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7057      +/-   ##
==========================================
- Coverage   67.44%   67.44%   -0.01%     
==========================================
  Files        3444     3445       +1     
  Lines       67865    67886      +21     
  Branches    11027    11037      +10     
==========================================
+ Hits        45772    45783      +11     
- Misses      19429    19434       +5     
- Partials     2664     2669       +5     
Flag Coverage Δ
Linux_1 33.07% <0.00%> (ø)
Linux_2 55.11% <ø> (ø)
Linux_3 45.25% <ø> (+0.01%) ⬆️
Linux_4 ?
Windows_1 33.10% <0.00%> (ø)
Windows_2 55.06% <ø> (ø)
Windows_3 45.25% <ø> (+0.01%) ⬆️
Windows_4 34.86% <0.00%> (ø)

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.

@kavilla
Copy link
Member

kavilla commented Jun 20, 2024

is the proposal within OSD or are these examples for potentially having the default values in OUI be smaller? so it hits the entire app?

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.

We probably need to follow up with 1) strategy on which changes will need to be behind a flag, 2) whether popover defaults should be made at OUI level and 3) how to address small context menu titles being a smaller size than popover titles even though body text size is the same, but changes lgtm and these could be addressed after.

@@ -179,13 +179,13 @@ class FilterOptionsUI extends Component<Props, State> {
panelPaddingSize="none"
repositionOnScroll
>
<EuiPopoverTitle>
<EuiPopoverTitle paddingSize="s">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think its better to do these locally rather than set a default through OUI?

Copy link
Collaborator Author

@AMoo-Miki AMoo-Miki Jun 28, 2024

Choose a reason for hiding this comment

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

Due to the potential blast radius, I think we should localize these changes. I am also concerned about different fonts and font-sizes across themes and their impact on all of these. did the math and the diff is too little to be concerned about.

@AMoo-Miki
Copy link
Collaborator Author

is the proposal within OSD or are these examples for potentially having the default values in OUI be smaller? so it hits the entire app?

There is a big collection of changes. Some are done globally within OUI (via a new theme) and some are done surgically. There will most likely be some that will go behind a config flag too.

@virajsanghvi virajsanghvi added the look & feel Look and Feel Improvements label Jul 10, 2024
@zhongnansu zhongnansu merged commit f1be0d6 into opensearch-project:main Jul 16, 2024
68 of 72 checks passed
@Hailong-am
Copy link
Contributor

opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 17, 2024
* Condense share context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense console editor actions context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense filter bar context menus

Signed-off-by: Miki <miki@amazon.com>

* Condense discover table options context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense new panel creation context menus

Signed-off-by: Miki <miki@amazon.com>

* Condense index pattern creation button's menu

Signed-off-by: Miki <miki@amazon.com>

* Condense index pattern creation wizard's pagination context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense inspect flyout's view selection context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense inspect flyout's download options context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense inspect flyout's request selection context menu

Signed-off-by: Miki <miki@amazon.com>

* Condense panel-adding flyout's filter menus

Signed-off-by: Miki <miki@amazon.com>

* Condense group adding context menu in visualization editor

Signed-off-by: Miki <miki@amazon.com>

---------

Signed-off-by: Miki <miki@amazon.com>
(cherry picked from commit f1be0d6)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
zhongnansu pushed a commit that referenced this pull request Jul 18, 2024
* Condense share context menu



* Condense console editor actions context menu



* Condense filter bar context menus



* Condense discover table options context menu



* Condense new panel creation context menus



* Condense index pattern creation button's menu



* Condense index pattern creation wizard's pagination context menu



* Condense inspect flyout's view selection context menu



* Condense inspect flyout's download options context menu



* Condense inspect flyout's request selection context menu



* Condense panel-adding flyout's filter menus



* Condense group adding context menu in visualization editor



---------


(cherry picked from commit f1be0d6)

Signed-off-by: Miki <miki@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>
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.

5 participants