Skip to content

fix: 🐛 don't add separator before group on no main items#83166

Merged
streamich merged 3 commits intoelastic:masterfrom
streamich:fix-first-group-sep
Dec 3, 2020
Merged

fix: 🐛 don't add separator before group on no main items#83166
streamich merged 3 commits intoelastic:masterfrom
streamich:fix-first-group-sep

Conversation

@streamich
Copy link
Copy Markdown
Contributor

@streamich streamich commented Nov 11, 2020

Summary

This change stops adding a menu separator line before an action grouping if there is nothing in the menu, this prevents the separator to be the very first item in the menu.

Checklist

Delete any items that are not applicable to this PR.

@streamich streamich added Feature:UIActions UI actions. These are client side only, not related to the server side actions.. release_note:skip Skip the PR/issue when compiling release notes Team:AppArch v7.11.0 v8.0.0 labels Nov 11, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 11, 2020

Pinging @elastic/kibana-app-arch (Team:AppArch)

tsullivan
tsullivan approved these changes Nov 11, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

I would like to see a screenshot in a PR. Other than that, the code changes LGTM

@streamich
Copy link
Copy Markdown
Contributor Author

streamich commented Nov 20, 2020

@elasticmachine merge upstream

@kibanamachine kibanamachine requested a review from a team as a code owner November 20, 2020 16:12
@streamich
Copy link
Copy Markdown
Contributor Author

streamich commented Dec 3, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 3, 2020

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
uiActions 67.7KB 67.8KB +35.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Dosant
Dosant approved these changes Dec 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Code LGTM!

@streamich streamich merged commit bb76989 into elastic:master Dec 3, 2020
@streamich streamich mentioned this pull request Dec 3, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge branch 'master' into alerts/force-schedule
ffcc5ca 
* master: (40 commits)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992)
  [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge branch 'master' into alerts/flaky-event-log-tests
5cce0f7 
* master: (236 commits)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992)
  [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge remote-tracking branch 'upstream/master' into alerts/custom-rec…
7762864 
…overy-action-group

* upstream/master: (48 commits)
  [Lens] accessibility screen reader issues (elastic#84395)
  [Logs UI] Fetch single log entries via a search strategy (elastic#81710)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  ...
streamich added a commit that referenced this pull request Dec 3, 2020
@streamich @kibanamachine
fix: 🐛 don't add separator befor group on no main items (#83166) (#84881
4d19cab 
)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsullivan tsullivan tsullivan approved these changes

@Dosant Dosant Dosant approved these changes

Assignees

No one assigned

Labels

Feature:UIActions UI actions. These are client side only, not related to the server side actions.. release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@streamich @elasticmachine @kibanamachine @tsullivan @Dosant