Legend, Chart styles, and Color Theme features implementation for Bar chart on config panel - 697 #4
Conversation
…for Bar chart Signed-off-by: rinku-kumar-psl <[email protected]>
Signed-off-by: rinku-kumar-psl <[email protected]>
…olved Signed-off-by: rinku-kumar-psl <[email protected]>
…endered rotate label UI Signed-off-by: rinku-kumar-psl <[email protected]>
Signed-off-by: rinku-kumar-psl <[email protected]>
Signed-off-by: rinku-kumar-psl <[email protected]>
Signed-off-by: rinku-kumar-psl <[email protected]>
| @@ -186,9 +186,11 @@ export interface IConfigPanelOptions { | |||
| export interface IConfigPanelOptionSection { | |||
| name: string; | |||
| component: null; | |||
| mapTo: 'mode'; | |||
| mapTo: 'mode' | string; | |||
There was a problem hiding this comment.
isn't only string going to be sufficient here? 'mode' is also a string.
| const { data: vizData = {}, metadata: { fields = [] } = {} } = data?.rawVizData; | ||
|
|
||
| const handleConfigurationChange = useCallback( | ||
| (stateFiledName) => { |
There was a problem hiding this comment.
Did you mean field or filled? Looks like there is a typo.
| }, [vizState]); | ||
|
|
||
| const dimensions = useMemo(() => | ||
| currentSchemas.map((schema: IConfigPanelOptionSection, index: string) => { |
There was a problem hiding this comment.
index in map function is always an integer.
| }; | ||
| } else if (schema.eleType === 'slider') { | ||
| params = { | ||
| minRange: schema?.props?.min, |
There was a problem hiding this comment.
What is going to happen, if either schema or props is not available? As you mentioned, they are optional here and it may happen that minRange may be undefined. Are we handling that situation?
Same is applicable for maxRange as well.
There was a problem hiding this comment.
Thanks, Subrat,
For minRange its handled if it's undefined or not passed, EUI Range component will take default minRange as 0. But we have to make maxRange as required filed for slider. So made the required changes.
| title: schema.name, | ||
| currentRange: vizState[schema.mapTo] || schema?.defaultState, | ||
| ticks: schema?.props?.ticks, | ||
| showTicks: schema?.props?.showTicks, |
There was a problem hiding this comment.
Similar to above, showTicks should be assigned some value, incase the schema.props does not exist. As you have marked them optional, some default value should be provided.
There was a problem hiding this comment.
Here as we made the slider component to work as both plain Slider range and tick mark slider range, So If 'ticks' and 'showTicks' will be undefined(falsy value) then this will work as Plain Slider Range. Please suggest if this approach is correct or not.
| let res = vizState; | ||
| if (isEmpty(vizState)) res = []; |
There was a problem hiding this comment.
const res = isEmpty(vizState)? []: vizState;
| (ctId, ctName) => { | ||
| return (event) => { |
There was a problem hiding this comment.
(ctId, ctName) => event => {.........} You may not the return statement here.
| (ctId) => { | ||
| return (event) => { |
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import React, { useCallback } from 'react'; |
There was a problem hiding this comment.
Looks like the new files that you have created (like this) have a tab stop of 4 spaces. The existing files have tab length as 2 spaces. Could you please check once?
| const barWidth = 1 - (dataConfig?.chartStyles?.barWidth || vis.barWidth); | ||
| const groupWidth = 1 - (dataConfig?.chartStyles?.groupWidth || vis.groupWidth); | ||
| const isVertical = barOrientation === vis.orientation; | ||
| const showLegend = dataConfig?.legend?.showLegend && dataConfig.legend.showLegend !== vis.showLegend ? false : true; |
There was a problem hiding this comment.
This should work. We usually do not assign true or false after condition. The condition itself returns a boolean value.
const showLegend = !(dataConfig?.legend?.showLegend && dataConfig.legend.showLegend !== vis.showLegend);There was a problem hiding this comment.
Yes, it's working. Thanks!
Signed-off-by: rinku-kumar-psl <[email protected]>
Signed-off-by: rinku-kumar-psl <[email protected]>
|
Raising external PR, so closing this one. |
Description
[Describe what this change achieves]
The below features are implemented in the config panel in this PR:
Legend:
Chart styles:
Color Theme
Here are the visuals and implementation of new Data configs on the Bar chart as per the issue opensearch-project#697
opensearch-project#697 (comment)
Issues Resolved
[List any issues this PR will resolve]
opensearch-project#697
Check List
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.