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

Sprint3 #911

Closed
wants to merge 32 commits into from
Closed

Sprint3 #911

wants to merge 32 commits into from

Conversation

abasatwar
Copy link
Contributor

@abasatwar abasatwar commented Aug 2, 2022

Description

Visualization implementatation for multiple charts and few bugs are fixed in this PR.

Issues Resolved
Filter Logs View data #863
[BUG] : In Gauge Chart, number of gauges in Data field can be set to negative value. #892
Logs view implementation for Visualization #773
[BUG]: Naming convention for charts field are not common for all charts #895
[FEATURE]: Scatter Chart new Config Options Implementation #823
[BUG]: Add update chart button for data configuration panel #887

Check List

  • New functionality includes testing.
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

abasatwar and others added 17 commits July 15, 2022 19:37
* graph style section UI schema

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

* changes for style mode and interpolation

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

* lineWidth integration for line mode

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* changes for Legend and Orientation in Line

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_button_group.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_legend.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* point size and Bar Alignment changes

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/common/types/explorer.ts

* implemented fill opacity for line chart

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

* changes for line width and fill opacity in bar mode and removed mode from chartOption

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/common/constants/shared.ts

* updated bar mode opacity in line chart

Signed-off-by: rinku-kumar-psl <[email protected]>

* refactored the config chart style code

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/common/types/explorer.ts

* snapshot updated and code refactored

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

* type added to new component

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_button_group.tsx

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* review comments addressed

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

# Conflicts:
#	dashboards-observability/common/constants/shared.ts

* cypress test case added and resolve button label wraping issue

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js
#	dashboards-observability/.cypress/utils/event_constants.js

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js

* multi matrices changes for Line

Signed-off-by: rinku-kumar-psl <[email protected]>

* dimensions and metrics UI changes for time-series

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/common/constants/explorer.ts
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/index.tsx

* made data config pannel collapsable and initial fields render

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/index.tsx

* code refactored

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx

* snapshot updated and handled corner cases

Signed-off-by: rinku-kumar-psl <[email protected]>

* code styling fixes and added TODO comment

Signed-off-by: rinku-kumar-psl <[email protected]>

* sequence change for dimensions and metrics

Signed-off-by: Deepak Nevde <[email protected]>

* Sprint1 (#14)

* graph style section UI schema

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

* changes for style mode and interpolation

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

* lineWidth integration for line mode

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* changes for Legend and Orientation in Line

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_button_group.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_legend.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* point size and Bar Alignment changes

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx
#	dashboards-observability/public/components/visualizations/charts/lines/line_type.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/common/types/explorer.ts

* implemented fill opacity for line chart

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/visualizations/charts/lines/line.tsx

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

* changes for line width and fill opacity in bar mode and removed mode from chartOption

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/common/constants/shared.ts

* updated bar mode opacity in line chart

Signed-off-by: rinku-kumar-psl <[email protected]>

* refactored the config chart style code

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/index.ts

# Conflicts:
#	dashboards-observability/common/constants/shared.ts
#	dashboards-observability/common/types/explorer.ts

* snapshot updated and code refactored

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

* type added to new component

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_button_group.tsx

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/config_style_slider.tsx

* review comments addressed

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/utils/utils.tsx

# Conflicts:
#	dashboards-observability/common/constants/shared.ts

* cypress test case added and resolve button label wraping issue

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js
#	dashboards-observability/.cypress/utils/event_constants.js

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js

# Conflicts:
#	dashboards-observability/.cypress/integration/1_event_analytics.spec.js

* multi matrices changes for Line

Signed-off-by: rinku-kumar-psl <[email protected]>

* dimensions and metrics UI changes for time-series

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/common/constants/explorer.ts
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/index.tsx

* made data config pannel collapsable and initial fields render

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/index.tsx

* code refactored

Signed-off-by: rinku-kumar-psl <[email protected]>

# Conflicts:
#	dashboards-observability/public/components/event_analytics/explorer/visualizations/config_panel/config_panes/config_controls/data_config_panel_item.tsx

* snapshot updated and handled corner cases

Signed-off-by: rinku-kumar-psl <[email protected]>

* code styling fixes and added TODO comment

Signed-off-by: rinku-kumar-psl <[email protected]>

* data config reviewed code added

Signed-off-by: Deepak Nevde <[email protected]>

* Text correction

Signed-off-by: Deepak Nevde <[email protected]>

* Conflicts resolved

Signed-off-by: Deepak Nevde <[email protected]>

* table view: eui table replaced with ag-grid

Signed-off-by: Ramneet Chopra <[email protected]>

* drag-drop issue fixed

Signed-off-by: Ramneet Chopra <[email protected]>

* test case of data_table updated

Signed-off-by: Ramneet Chopra <[email protected]>

* feedback comments resolved

Signed-off-by: Ramneet Chopra <[email protected]>

* grid height issue:fixed

Signed-off-by: Ramneet Chopra <[email protected]>

* column height, value getter for type double

Signed-off-by: Ramneet Chopra <[email protected]>

* data_table elements moved to separate

Signed-off-by: Ramneet Chopra <[email protected]>

* footer components

Signed-off-by: Ramneet Chopra <[email protected]>

* cypress test cases for table view

Signed-off-by: Ramneet Chopra <[email protected]>

* enhancement for heatmap with new UI

Signed-off-by: Shankha Das <[email protected]>

* line chart test cases

Signed-off-by: Shankha Das <[email protected]>

* console logs removed

Signed-off-by: Shankha Das <[email protected]>

* updated value options ui for treemap

Signed-off-by: Mrunal Zambre <[email protected]>

* removed console

Signed-off-by: Mrunal Zambre <[email protected]>

* Fixes of sprint1 for new ui implementation (#12)

Signed-off-by: ruchika-narang <[email protected]>

Co-authored-by: rinku-kumar-psl <[email protected]>
Co-authored-by: Deepak Nevde <[email protected]>
Co-authored-by: Ramneet Chopra <[email protected]>
Co-authored-by: Shankha Das <[email protected]>
Co-authored-by: Mrunal Zambre <[email protected]>
Co-authored-by: ruchika-narang <[email protected]>

* Latest code added

Signed-off-by: Deepak Nevde <[email protected]>

* Collapsapable button position change to top

Signed-off-by: Deepak Nevde <[email protected]>

* table view: eui table replaced with ag-grid

Signed-off-by: Ramneet Chopra <[email protected]>

* drag-drop issue fixed

Signed-off-by: Ramneet Chopra <[email protected]>

* test case of data_table updated

Signed-off-by: Ramneet Chopra <[email protected]>

* feedback comments resolved

Signed-off-by: Ramneet Chopra <[email protected]>

* grid height issue:fixed

Signed-off-by: Ramneet Chopra <[email protected]>

* column height, value getter for type double

Signed-off-by: Ramneet Chopra <[email protected]>

* data_table elements moved to separate

Signed-off-by: Ramneet Chopra <[email protected]>

* footer components

Signed-off-by: Ramneet Chopra <[email protected]>

* cypress test cases for table view

Signed-off-by: Ramneet Chopra <[email protected]>

* data config reviewed code added

Signed-off-by: Deepak Nevde <[email protected]>

* Text correction

Signed-off-by: Deepak Nevde <[email protected]>

* Conflicts resolved

Signed-off-by: Deepak Nevde <[email protected]>

* enhancement for heatmap with new UI

Signed-off-by: Shankha Das <[email protected]>

* line chart test cases

Signed-off-by: Shankha Das <[email protected]>

* console logs removed

Signed-off-by: Shankha Das <[email protected]>

* updated value options ui for treemap

Signed-off-by: Mrunal Zambre <[email protected]>

* removed console

Signed-off-by: Mrunal Zambre <[email protected]>

* sprint1-visualization-fixes.

Signed-off-by: abasatwar <[email protected]>

* added colorscale config options for treemap

Signed-off-by: Mrunal Zambre <[email protected]>

* code review comment resolved

Signed-off-by: Deepak Nevde <[email protected]>

* added config option to sort treemap sectors

Signed-off-by: Mrunal Zambre <[email protected]>

* changes to resctct duplicte options

Signed-off-by: rinku-kumar-psl <[email protected]>

* Updated and Added test scripts for Treemap chart along with data config and worked on reassembling the event_constants.js file

Signed-off-by: Pratibha Pandey <[email protected]>

* Removed unwanted code

Signed-off-by: Pratibha Pandey <[email protected]>

* implementation of histogram with new UI

Signed-off-by: ruchika-narang <[email protected]>

* gauge chart added

Signed-off-by: Ramneet Chopra <[email protected]>

* Pie chart enhancement, multi labels change

Signed-off-by: Deepak Nevde <[email protected]>

* threshold text fix

Signed-off-by: Ramneet Chopra <[email protected]>

* cypress test cases added

Signed-off-by: Ramneet Chopra <[email protected]>

* Code review comment resolved

Signed-off-by: Deepak Nevde <[email protected]>

* reset fixed, unused imports removed, PR checks fixed

Signed-off-by: Ramneet Chopra <[email protected]>

* single timestamp dimension, no data dsiplay, label rotate, label/legend size

Signed-off-by: Ramneet Chopra <[email protected]>

* layout fixed for primary y axis

Signed-off-by: Ramneet Chopra <[email protected]>

* change of screen size of no data found and visualization

Signed-off-by: Shankha Das <[email protected]>

* changes for restriction of duplicate fields on Data Config and only numeric field selection to metrics

Signed-off-by: rinku-kumar-psl <[email protected]>

* line label replaced with time series

Signed-off-by: Ramneet Chopra <[email protected]>

* snapshot tests

Signed-off-by: Ramneet Chopra <[email protected]>

* Removed unwanted spaces

Signed-off-by: Pratibha Pandey <[email protected]>

* initialize default params for DimensonComponent and formatted the codes

Signed-off-by: rinku-kumar-psl <[email protected]>

* pr review feedback

Signed-off-by: Ramneet Chopra <[email protected]>

* updated preview functionality for charts

Signed-off-by: Mrunal Zambre <[email protected]>

* updated snapshots

Signed-off-by: Mrunal Zambre <[email protected]>

* Worked on review comments

Signed-off-by: Pratibha Pandey <[email protected]>

* changed variable names

Signed-off-by: Mrunal Zambre <[email protected]>

* code review changes done

Signed-off-by: rinku-kumar-psl <[email protected]>

* added empty line at end.

Signed-off-by: abasatwar <[email protected]>

* updated variable names

Signed-off-by: Mrunal Zambre <[email protected]>

* updated snapshots

Signed-off-by: Mrunal Zambre <[email protected]>

* Added pie chart test cases

Signed-off-by: Pratibha Pandey <[email protected]>

* updated snapshots

Signed-off-by: ruchika-narang <[email protected]>

* Removed consoles

Signed-off-by: ruchika-narang <[email protected]>

* Worked on conflicts

Signed-off-by: Pratibha Pandey <[email protected]>

* color selector added

Signed-off-by: Ramneet Chopra <[email protected]>

* updated snapshots

Signed-off-by: ruchika-narang <[email protected]>

* UI updated as recommended

Signed-off-by: Shankha Das <[email protected]>

* Added legend to heatmap

Signed-off-by: ruchika-narang <[email protected]>

* data_config_panel_item timeseries code removed

Signed-off-by: Ramneet Chopra <[email protected]>

* bar chart with multiple dimension and metrics, timestamp

Signed-off-by: abasatwar <[email protected]>

* limit no. of gauge option added

Signed-off-by: Ramneet Chopra <[email protected]>

* threshold limit added, gauge default parameters moved to constants/explorer

Signed-off-by: Ramneet Chopra <[email protected]>

* legend placement added

Signed-off-by: Ramneet Chopra <[email protected]>

* yarn test

Signed-off-by: Ramneet Chopra <[email protected]>

* snapshot updated

Signed-off-by: abasatwar <[email protected]>

* snapshot updated

Signed-off-by: abasatwar <[email protected]>

* Resolving issues after removal of preview functionality

Signed-off-by: ruchika-narang <[email protected]>

* updated snapshots

Signed-off-by: ruchika-narang <[email protected]>

* Updated snapshots

Signed-off-by: ruchika-narang <[email protected]>

* changes for default timestamp data for time-series and corner cases

Signed-off-by: rinku-kumar-psl <[email protected]>

* code review comment addressed

Signed-off-by: rinku-kumar-psl <[email protected]>

* pr feedback

Signed-off-by: Ramneet Chopra <[email protected]>

* dimensions, metrics length checks refined

Signed-off-by: Ramneet Chopra <[email protected]>

* updated as per review comments

Signed-off-by: abasatwar <[email protected]>

* fixing of data config corner cases

Signed-off-by: rinku-kumar-psl <[email protected]>

* snapshot updated

Signed-off-by: abasatwar <[email protected]>

* pr feedback

Signed-off-by: Ramneet Chopra <[email protected]>

Co-authored-by: rinku-kumar-psl <[email protected]>
Co-authored-by: Deepak Nevde <[email protected]>
Co-authored-by: Ramneet Chopra <[email protected]>
Co-authored-by: Shankha Das <[email protected]>
Co-authored-by: Mrunal Zambre <[email protected]>
Co-authored-by: ruchika-narang <[email protected]>
Co-authored-by: Pratibha Pandey <[email protected]>
Co-authored-by: ruchika-narang <[email protected]>
Co-authored-by: Shankha Das <[email protected]>
Bug: Add update chart button for data configuration
…scatter-823

Implemented Scatter visualization
pr issue fixed for min limit in gauge input, tooltip added for thresh…
Signed-off-by: Shankha Das <[email protected]>
Signed-off-by: Deepak Nevde <[email protected]>
Signed-off-by: Shankha Das <[email protected]>
Signed-off-by: Shankha Das <[email protected]>
Feature/logs-view-new: Updated the Logs View UI
@abasatwar abasatwar requested a review from a team as a code owner August 2, 2022 12:49
@abasatwar
Copy link
Contributor Author

hi @mengweieric ,
As you requested to earlier , here is list of PR(Internal) where above code is reviewed individually.
abasatwar#61
abasatwar#58
abasatwar#55
abasatwar#56
abasatwar#54
And code is combined to avoid multiple conflicts.
CC: @spattnaik , @anirudha

Thanks,
Amruta

return ENABLED_VIS_TYPES.map((vs: string) => {
const visDefinition = getVisType(vs);
if (vs === visChartTypes.Line || vs === visChartTypes.Scatter) {
visDefinition = vs === visChartTypes.Line ? getVisType(vs, { type: visChartTypes.Line }) : getVisType(vs, { type: visChartTypes.Scatter });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, this can be done by the following. No need for an additional if condition.

visDefinition = getVisType(vs, { type: vs });

const visDefinition = getVisType(vs);
if (vs === visChartTypes.Line || vs === visChartTypes.Scatter) {
visDefinition = vs === visChartTypes.Line ? getVisType(vs, { type: visChartTypes.Line }) : getVisType(vs, { type: visChartTypes.Scatter });
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

After the above been done, the complete function can also be reduced to

const visDefinition = ENABLED_VIS_TYPES.map((vs: string) => vs === visChartTypes.Line || vs === visChartTypes.Scatter 
  ? getVisType(vs, { type: vs })
  : getVisType(vs)

@@ -33,9 +34,18 @@ export const ConfigLineChartStyles = ({

/* To update the schema options based on current style mode selection */
const currentSchemas = useMemo(() => {
if (!vizState?.style || vizState?.style === 'lines') {
if (!vizState?.style) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The complete block can be re-written as:

if (vizState?.style) {
  switch(vizState.style) {
    "lines": .....;
    "bar": .....;
  }
} else {
  if (visualizations?.vis?.name === visChartTypes.Scatter) { ...... }
}

Copy link
Contributor

@spattnaik spattnaik left a comment

Choose a reason for hiding this comment

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

Few more comments, please take a look.

@@ -45,7 +46,7 @@ export const ConfigPanelOptionGauge = ({
handleConfigChange(newPanelOptions);
}}
placeholder={'Number of gauges'}
readOnly={isReadOnly}
readOnly={!dimensions.length}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think dimensions.length === 0 is more readable?

const AddButtonTextWrapper = () =>
props?.maxLimit && !isEmpty(vizState) && vizState.length === props.maxLimit ? (
<EuiToolTip position="top" content="Only one threshold can be applied">
<>{addButtonText}</>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need this additional fragment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually tooltip accepts JSX elements only, that's why we have to wrap it using Fragment.

Comment on lines +189 to +191
? unselectedFields.filter((i) => i.type === 'timestamp')
: unselectedFields;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the indent was correct previously.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried indenting it and used prettify then. It's getting back to this indentation.

Comment on lines 64 to 66
return vizId === visChartTypes.Line
? { ...getVisType(vizId, { type: visChartTypes.Line }) }
: { ...getVisType(vizId, { type: visChartTypes.Scatter }) };
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to one of the comments given in another file. Please check how the code can be reduced here and with better readability.

deepaknevdepsl and others added 3 commits August 3, 2022 12:14
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2022

Codecov Report

Merging #911 (d53a2e1) into main (b5c6802) will decrease coverage by 0.10%.
The diff coverage is 47.00%.

@@             Coverage Diff              @@
##               main     #911      +/-   ##
============================================
- Coverage     55.59%   55.48%   -0.11%     
  Complexity      291      291              
============================================
  Files           260      262       +2     
  Lines          8715     8761      +46     
  Branches       1892     1922      +30     
============================================
+ Hits           4845     4861      +16     
- Misses         3699     3729      +30     
  Partials        171      171              
Flag Coverage Δ
dashboards-observability 49.61% <47.00%> (-0.11%) ⬇️
opensearch-observability 71.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...hboards-observability/common/constants/explorer.ts 100.00% <ø> (ø)
.../public/components/custom_panels/helpers/utils.tsx 82.85% <ø> (ø)
...fig_panes/config_controls/config_chart_options.tsx 1.72% <ø> (ø)
...onfig_panes/config_controls/config_color_theme.tsx 50.00% <0.00%> (ø)
...panes/config_controls/config_line_chart_styles.tsx 1.96% <0.00%> (-0.43%) ⬇️
...nfig_panes/config_controls/config_number_input.tsx 66.66% <ø> (ø)
...anes/config_controls/config_panel_option_gauge.tsx 11.76% <0.00%> (-0.74%) ⬇️
...config_panes/config_controls/config_thresholds.tsx 2.85% <0.00%> (-0.37%) ⬇️
...c/components/visualizations/charts/bar/bar_type.ts 100.00% <ø> (ø)
...isualizations/charts/financial/gauge/gauge_type.ts 100.00% <ø> (ø)
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Shankha Das <[email protected]>
Signed-off-by: Shankha Das <[email protected]>
spattnaik
spattnaik previously approved these changes Aug 3, 2022
Copy link
Contributor

@spattnaik spattnaik left a comment

Choose a reason for hiding this comment

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

Looks good

Signed-off-by: Shankha Das <[email protected]>
@abasatwar
Copy link
Contributor Author

Raised new PR - #917

Copy link
Contributor Author

@abasatwar abasatwar left a comment

Choose a reason for hiding this comment

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

looks good.

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

Successfully merging this pull request may close these issues.

7 participants