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

feat: add tooltip to the tablecard column and value card label #3871

Merged
merged 3 commits into from
Jun 24, 2024
Merged

feat: add tooltip to the tablecard column and value card label #3871

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f4f934
feat: add tooltip to the tablecard column
herleraja Jun 24, 2024
b77cfdb
chore: adding amrith as a code owner to get PR reviews
herleraja Jun 24, 2024
41a1d6a
test(tablecard): revert tablecard tooltip test due to dashboard grid …
herleraja Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,30 @@
# These users own any files in the following directory at the root of
# the repository and any of its subdirectories.

* @davidicus @jessieyan @sls-ca
* @davidicus @jessieyan @sls-ca @herleraja

#####
# Core admin team should be notified of changes to build/test/deploy

# Core dependencies
/package.json @davidicus @jessieyan @sls-ca
/yarn.lock @davidicus @jessieyan @sls-ca
/package.json @davidicus @jessieyan @sls-ca @herleraja
/yarn.lock @davidicus @jessieyan @sls-ca @herleraja

# Configuration files
**/*.config.js @davidicus @jessieyan @sls-ca
**/config/ @davidicus @jessieyan @sls-ca
/.nvmrc @davidicus @jessieyan @sls-ca
**/*.config.js @davidicus @jessieyan @sls-ca @herleraja
**/config/ @davidicus @jessieyan @sls-ca @herleraja
/.nvmrc @davidicus @jessieyan @sls-ca @herleraja

# Deploy configuration
**/.storybook/ @davidicus @jessieyan @sls-ca
/.github/workflows/ @davidicus @jessieyan @sls-ca
/netlify.toml @davidicus @jessieyan @sls-ca
**/.storybook/ @davidicus @jessieyan @sls-ca @herleraja
/.github/workflows/ @davidicus @jessieyan @sls-ca @herleraja
/netlify.toml @davidicus @jessieyan @sls-ca @herleraja

#####
# Release team should be notified of Public API changes in the system

**/publicAPI.test.js @davidicus @jessieyan @sls-ca
**/publicAPI.test.js.snap @davidicus @jessieyan @sls-ca
**/publicAPI.test.js @davidicus @jessieyan @sls-ca @herleraja
**/publicAPI.test.js.snap @davidicus @jessieyan @sls-ca @herleraja

# Angular team should be required for Angular changes
/packages/angular/ @carbon-design-system/angular-maintainers

# Monitor still has use for this component
/packages/react/src/components/DashboardEditor/ @davidicus @jessieyan @sls-ca @herleraja
113 changes: 82 additions & 31 deletions ...eact/src/components/CardEditor/CardEditForm/CardEditFormItems/DataSeriesFormItemModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ const propTypes = {
value: PropTypes.oneOfType([PropTypes.bool, PropTypes.number, PropTypes.string]),
})
),
tooltip: PropTypes.string,
}),
setEditDataItem: PropTypes.func,
/** an object where the keys are available dimensions and the values are the values available for those dimensions
Expand Down Expand Up @@ -147,6 +148,14 @@ const propTypes = {
* Used to override the default behaviour of handleDataItemEdit. if we dont pass any function then it uses handleDefaultDataItemEdit function by default
*/
handleDataItemEdit: PropTypes.func,

options: PropTypes.shape({
hasColorDropdown: PropTypes.bool,
hasUnit: PropTypes.bool,
hasDecimalPlacesDropdown: PropTypes.bool,
hasThresholds: PropTypes.bool,
hasTooltip: PropTypes.bool,
}),
};

const defaultProps = {
Expand All @@ -170,6 +179,7 @@ const defaultProps = {
dataItemEditorDataItemFilter: 'Data filter',
dataItemEditorDataItemThresholds: 'Thresholds',
dataItemEditorDataItemAddThreshold: 'Add threshold',
dataItemEditorDataItemTooltip: 'Tooltip',
dataItemEditorBarColor: 'Bar color',
dataItemEditorLineColor: 'Line color',
dataItemEditorAddAggregationMethodLabel: 'Add aggregation method',
Expand Down Expand Up @@ -208,6 +218,13 @@ const defaultProps = {
},
},
handleDataItemEdit: handleDefaultDataItemEdit,
options: {
hasColorDropdown: false,
hasUnit: false,
hasDecimalPlacesDropdown: false,
hasThresholds: false,
hasTooltip: false,
},
};

const DATAITEM_COLORS_OPTIONS = [
Expand Down Expand Up @@ -243,10 +260,18 @@ const DataSeriesFormItemModal = ({
dataSeriesFormActions: { hasAggregationsDropDown, onAddAggregations, hasDataFilterDropdown },
},
handleDataItemEdit,
options,
}) => {
const mergedI18n = { ...defaultProps.i18n, ...i18n };
const { id, type, content } = cardConfig;
const baseClassName = `${iotPrefix}--card-edit-form`;
const {
hasColorDropdown,
hasUnit,
hasDecimalPlacesDropdown,
hasThresholds,
hasTooltip,
} = options;

const isTimeBasedCard =
type === CARD_TYPES.TIMESERIES ||
Expand All @@ -266,8 +291,6 @@ const DataSeriesFormItemModal = ({
decrementNumberText,
} = mergedI18n;
switch (idToTranslate) {
default:
return '';
case 'clear.all':
return clearAllText || 'Clear all';
case 'clear.selection':
Expand All @@ -280,6 +303,8 @@ const DataSeriesFormItemModal = ({
return incrementNumberText || 'Increment number';
case 'decrement.number':
return decrementNumberText || 'Decrement number';
default:
return '';
}
},
[mergedI18n]
Expand Down Expand Up @@ -427,7 +452,7 @@ const DataSeriesFormItemModal = ({
/>
</div>

{(type === CARD_TYPES.TIMESERIES || type === CARD_TYPES.BAR) && ( // show color selector
{hasColorDropdown && ( // show color selector
<div className={`${baseClassName}--input-group--item`}>
<ColorDropdown
id={`${id}_color-dropdown`}
Expand All @@ -449,47 +474,68 @@ const DataSeriesFormItemModal = ({
/>
</div>
)}
</div>

{(type === CARD_TYPES.VALUE || type === CARD_TYPES.IMAGE) && (
<div className={`${baseClassName}--input-group`}>
{hasTooltip && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest we keep the previous validation, by just deleting that, we can impact others that use this component, and maybe add deprecation if we are removing the type prop

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have deleted here. But in the parent component ,I kept it same. So functionality wise there is no change.
image

<div className={`${baseClassName}--input-group--item`}>
<TextInput
id={`${id}_attribute-unit`}
labelText={mergedI18n.dataItemEditorDataItemUnit}
id={`${id}_attribute-tooltip`}
labelText={mergedI18n.dataItemEditorDataItemTooltip}
herleraja marked this conversation as resolved.
Show resolved Hide resolved
light
placeholder={`${mergedI18n.example}: %`}
onChange={(evt) =>
setEditDataItem({
...editDataItem,
unit: evt.target.value,
tooltip: evt.target.value,
})
}
value={editDataItem.unit}
value={editDataItem.tooltip}
herleraja marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
<div className={`${baseClassName}--input-group--item-end`}>
<Dropdown
id={`${id}_value-card-decimal-place`}
titleText={mergedI18n.decimalPlacesLabel}
direction="bottom"
label=""
items={[mergedI18n.notSet, '0', '1', '2', '3', '4']}
light
selectedItem={editDataItem.precision?.toString() || mergedI18n.notSet}
onChange={({ selectedItem }) => {
const isSet = selectedItem !== mergedI18n.notSet;
if (isSet) {
)}
</div>

{(hasUnit || hasDecimalPlacesDropdown) && (
<div className={`${baseClassName}--input-group`}>
{hasUnit && (
<div className={`${baseClassName}--input-group--item`}>
<TextInput
id={`${id}_attribute-unit`}
labelText={mergedI18n.dataItemEditorDataItemUnit}
light
placeholder={`${mergedI18n.example}: %`}
onChange={(evt) =>
setEditDataItem({
...editDataItem,
precision: Number(selectedItem),
});
} else {
setEditDataItem(omit(editDataItem, 'precision'));
unit: evt.target.value,
})
}
}}
/>
</div>
value={editDataItem.unit}
/>
</div>
)}
{hasDecimalPlacesDropdown && (
<div className={`${baseClassName}--input-group--item-end`}>
<Dropdown
id={`${id}_value-card-decimal-place`}
titleText={mergedI18n.decimalPlacesLabel}
direction="bottom"
label=""
items={[mergedI18n.notSet, '0', '1', '2', '3', '4']}
light
selectedItem={editDataItem.precision?.toString() || mergedI18n.notSet}
onChange={({ selectedItem }) => {
const isSet = selectedItem !== mergedI18n.notSet;
if (isSet) {
setEditDataItem({
...editDataItem,
precision: Number(selectedItem),
});
} else {
setEditDataItem(omit(editDataItem, 'precision'));
}
}}
/>
</div>
)}
</div>
)}

Expand Down Expand Up @@ -560,7 +606,7 @@ const DataSeriesFormItemModal = ({
</div>
)}

{(type === CARD_TYPES.VALUE || type === CARD_TYPES.IMAGE || type === CARD_TYPES.TABLE) && (
{hasThresholds && (
herleraja marked this conversation as resolved.
Show resolved Hide resolved
<ThresholdsFormItem
id={`${id}_thresholds`}
i18n={mergedI18n}
Expand All @@ -586,7 +632,12 @@ const DataSeriesFormItemModal = ({
editDataItem,
handleTranslation,
hasAggregationsDropDown,
hasColorDropdown,
hasDataFilterDropdown,
hasDecimalPlacesDropdown,
hasThresholds,
hasTooltip,
hasUnit,
id,
initialAggregation,
initialGrain,
Expand Down
33 changes: 33 additions & 0 deletions ...src/components/CardEditor/CardEditForm/CardEditFormItems/DataSeriesFormItemModal.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@ describe('DataSeriesFormItemModal', () => {
onAddAggregations: jest.fn(),
},
},
options: {
hasColorDropdown: true,
hasUnit: true,
hasDecimalPlacesDropdown: true,
hasThresholds: true,
hasTooltip: true,
},
};

it('Renders for timeseries card data', () => {
Expand All @@ -189,12 +196,38 @@ describe('DataSeriesFormItemModal', () => {
expect(label).toBeInTheDocument();
expect(legendColorLabel).toBeInTheDocument();

const tooltip = screen.getByText('Tooltip');
expect(tooltip).toBeInTheDocument();

const dataFilter = screen.getByRole('listbox', { name: 'Data filter' });
expect(dataFilter).toBeInTheDocument();

const aggregationDropdown = screen.getByRole('listbox', { name: 'Aggregation method' });
expect(aggregationDropdown).toBeInTheDocument();
});

it('Renders card data with out any options', () => {
render(
<DataSeriesFormItemModal
{...commonProps}
showEditor
cardConfig={valueCardConfig}
editDataItem={editTimeseriesDataItem}
editDataSeries={editDataSeriesTimeSeries}
options={undefined}
/>
);

const label = screen.queryByText('Custom label');
expect(label).toBeInTheDocument();

const unit = screen.queryByText('Unit');
expect(unit).not.toBeInTheDocument();

const tooltip = screen.queryByText('Tooltip');
expect(tooltip).not.toBeInTheDocument();
});

it('Renders for timeseries card data without datafilter', () => {
render(
<DataSeriesFormItemModal
Expand Down
8 changes: 8 additions & 0 deletions ...s/CardEditor/CardEditForm/CardEditFormItems/DataSeriesFormItems/DataSeriesFormContent.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ const DataSeriesFormItem = ({
const [removedDataItems, setRemovedDataItems] = useState([]);

const canMultiSelectDataItems = cardConfig.content?.type !== BAR_CHART_TYPES.SIMPLE;
const { type } = cardConfig;

// determine which content section to look at
const dataSection =
Expand Down Expand Up @@ -467,6 +468,13 @@ const DataSeriesFormItem = ({
onChange={onChange}
i18n={mergedI18n}
actions={actions}
options={{
hasColorDropdown: type === CARD_TYPES.TIMESERIES || type === CARD_TYPES.BAR,
hasUnit: type === CARD_TYPES.VALUE,
hasDecimalPlacesDropdown: type === CARD_TYPES.VALUE,
hasThresholds: type === CARD_TYPES.VALUE,
hasTooltip: type === CARD_TYPES.VALUE,
}}
/>
<ContentFormItemTitle
title={mergedI18n.dataItemEditorSectionTitle}
Expand Down
7 changes: 7 additions & 0 deletions ...nts/CardEditor/CardEditForm/CardEditFormItems/TableCardFormItems/TableCardFormContent.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,13 @@ const TableCardFormContent = ({
onChange={handleDataItemModalChanges}
i18n={mergedI18n}
actions={actions}
options={{
hasColorDropdown: false,
hasUnit: false,
hasDecimalPlacesDropdown: false,
hasThresholds: true,
hasTooltip: true,
}}
/>
<ContentFormItemTitle
title={mergedI18n.tableColumnEditorSectionTitle}
Expand Down
7 changes: 7 additions & 0 deletions ...c/components/HotspotEditorModal/HotspotEditorDataSourceTab/HotspotEditorDataSourceTab.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,13 @@ const HotspotEditorDataSourceTab = ({
onChange={onChange}
i18n={mergedI18n}
actions={actions}
options={{
hasColorDropdown: false,
hasUnit: true,
hasDecimalPlacesDropdown: true,
hasThresholds: true,
hasTooltip: false,
}}
/>
<div className={`${baseClassName}--input`}>
<MultiSelect
Expand Down
Loading
Loading