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

Jump to bottom

[XY] [Lens] Adds opacity slider #100453

Merged
merged 8 commits into from
May 25, 2021
Merged

[XY] [Lens] Adds opacity slider #100453

merged 8 commits into from
May 25, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented May 24, 2021 •
edited
Loading

Summary

Closes #97226

Specifically:

  • Adds a fill opacity slider on Lens area charts
  • Adds a fill opacity slider on XY axis area charts
  • Allows the users to show/hide the dots on XY axis area charts
  • Adds a dots size slider for the line/area xy charts

Lens

Added a slider on the visual options menu. The slider is rendered only for area-specific charts
image

XY axis

Added a slider to control the opacity of the fill color. The slider is rendered only for area charts
image

Moreover, as there are users complaining about the dots that appear on the area charts I added two extra options:

  • Hide the dots
  • Change the size of the dots

image

(the dots size slider is disabled if the user has chosen to hide the dots)

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula changed the title [XY] Add opacity slider and dots size slider [XY] Adds opacity slider and dots size slider May 24, 2021
@stratoula stratoula changed the title [XY] Adds opacity slider and dots size slider [XY] [Lens] Adds opacity slider May 24, 2021
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label May 25, 2021
@stratoula stratoula marked this pull request as ready for review May 25, 2021 08:57
@stratoula stratoula requested a review from a team May 25, 2021 08:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

flash1293
flash1293 reviewed May 25, 2021
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

This is great, thanks for the addition! Just one minor nit-pick to make the UI smoother

x-pack/plugins/lens/public/xy_visualization/visual_options_popover/fill_opacity_option.tsx
value,
isFillOpacityEnabled = true,
}) => {
return isFillOpacityEnabled ? (
Copy link
Contributor

@flash1293 flash1293 May 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could we use x-pack/plugins/lens/public/shared_components/debounced_value.ts here? It's a flexible hook to debounce things like this so the chart doesn't re-render too often.

See

kibana/x-pack/plugins/lens/public/indexpattern_datasource/query_input.tsx

Line 29 in 15abf24

const { inputValue, handleInputChange } = useDebouncedValue({ value, onChange });
for an example usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! Sure

flash1293
flash1293 approved these changes May 25, 2021
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested and works just fine. Thank you so much!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 558 559 +1
visTypeXy 99 100 +1
total +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 154 155 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.1MB 1.1MB +1.8KB
visTypeXy 111.6KB 113.7KB +2.1KB
total +3.8KB

Page load bundle

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

id before after diff
visTypeXy 62.7KB 63.3KB +567.0B
Unknown metric groups

API count

id before after diff
lens 165 166 +1

History

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

@stratoula stratoula merged commit d8c2594 into elastic:master May 25, 2021
stratoula added a commit to stratoula/kibana that referenced this pull request May 25, 2021
@stratoula @kibanamachine
[XY] [Lens] Adds opacity slider (elastic#100453)
54a6074 
* [XY] Add opacity slider and dots size slider

* [Lens] Adds fill opacity slider

* Make the new sliders to appear fullwidth

* Change property name and fix unit tests

* Add a comment

* useDebouncedValue hook

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@stratoula stratoula mentioned this pull request May 25, 2021
Merged
stratoula added a commit that referenced this pull request May 25, 2021
@stratoula @kibanamachine
[XY] [Lens] Adds opacity slider (#100453) (#100568)
274ca39 
* [XY] Add opacity slider and dots size slider

* [Lens] Adds fill opacity slider

* Make the new sliders to appear fullwidth

* Change property name and fix unit tests

* Add a comment

* useDebouncedValue hook

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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 26, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into reporting/ua-…
812f82c 
…deprecation-ilm-policy

* 'master' of github.com:elastic/kibana: (101 commits)
  [ftr] migrate "docTable" service to FtrService class (elastic#100595)
  [ftr] migrate "listingTable" service to FtrService class (elastic#100606)
  Fixed comparing real value with formatted according to mode. (elastic#100456)
  [ftr] migrate "dataGrid" service to FtrService class (elastic#100593)
  [ftr] migrate "fieldEditor" to FtrService class (elastic#100597)
  [ftr] migrate "filterBar" service to FtrService class (elastic#100601)
  [triggersActionsUi] Reduce page load bundle to under 100kB (elastic#97770)
  [build] Clean jest configs (elastic#100594)
  refact(NA): remove extra pkg_npm target and add specific target folders for @kbn/analytics on Bazel (elastic#100569)
  Update dependency @elastic/charts to v29.2.0 (elastic#100587)
  [Maps] convert LayerPanel to typescript (elastic#100481)
  [Upgrade Assistant] Address copy feedback (elastic#99632)
  Open/Closed filter for observability alerts page (elastic#99217)
  One liner to expose the EQL query for debugging for users (elastic#100565)
  [KibanaPageLayout] Solution Nav specific styles & props (elastic#100089)
  [ftr] implement FtrService classes and migrate common services (elastic#99546)
  [XY] [Lens] Adds opacity slider (elastic#100453)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [DOCS] Remove redundant maps attribute (elastic#100426)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/lib/store/report_ilm_policy.ts
#	x-pack/plugins/reporting/server/lib/store/store.test.ts
#	x-pack/plugins/reporting/server/lib/store/store.ts
ecezalp pushed a commit to ecezalp/kibana that referenced this pull request May 26, 2021
@stratoula @kibanamachine @ecezalp
[XY] [Lens] Adds opacity slider (elastic#100453)
751d69e 
* [XY] Add opacity slider and dots size slider

* [Lens] Adds fill opacity slider

* Make the new sliders to appear fullwidth

* Change property name and fix unit tests

* Add a comment

* useDebouncedValue hook

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
ecezalp pushed a commit to ecezalp/kibana that referenced this pull request May 26, 2021
@stratoula @kibanamachine @ecezalp
[XY] [Lens] Adds opacity slider (elastic#100453)
5f663f5 
* [XY] Add opacity slider and dots size slider

* [Lens] Adds fill opacity slider

* Make the new sliders to appear fullwidth

* Change property name and fix unit tests

* Add a comment

* useDebouncedValue hook

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@stratoula stratoula mentioned this pull request Aug 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

Assignees
No one assigned
Labels
Feature:Lens Feature:XYAxis XY-Axis charts (bar, area, line) release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[XY axis] Give the ability to change the colors opacity
4 participants
@stratoula @elasticmachine @kibanamachine @flash1293