Skip to content

Commit

Permalink
[4077] Remove metadata from Charts widgets
Browse files Browse the repository at this point in the history
Bug: #4077
Signed-off-by: Axel RICHARD <[email protected]>
  • Loading branch information
AxelRICHARD authored and gcoutable committed Oct 10, 2024
1 parent 86b5b7a commit 8ae9424
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 98 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ The new option ALWAYS allows the separator to be displayed in every case.
- https://github.com/eclipse-sirius/sirius-web/issues/4050[#4050] [sirius-web] `IUploadFileLoader` must now return an `IResult`.
This allows an error to be displayed when there is a problem during uploading
- https://github.com/eclipse-sirius/sirius-web/issues/4037[#4037] [trees] The tree item id passed to `ITreeQueryService.findTreeItem` is no longer a UUID but is now a String.
- https://github.com/eclipse-sirius/sirius-web/issues/4077[#4077] [charts] Remove `interface RepresentationMetadata` from `BarChart.types.ts`.

=== Dependency update

Expand All @@ -56,6 +57,8 @@ This allows an error to be displayed when there is a problem during uploading
- https://github.com/eclipse-sirius/sirius-web/issues/4051[#4051] Error when deleting Resource that use EReference.isContainer feature
- https://github.com/eclipse-sirius/sirius-web/issues/3958[#3958] [trees] Prevent expand all action to loop indefinitely.
- https://github.com/eclipse-sirius/sirius-web/issues/4073[#4073] [trees] Add missing variables `ancestorIds` and `index` to variable manager used in the default expand all handler.
- https://github.com/eclipse-sirius/sirius-web/issues/4077[#4077] [charts] Remove metadata from Charts widgets.
Charts' metadata is only relevant when Charts are used as Representations.

=== New Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public final class BarChart implements IChart {

private int height;

private String yAxisLabel;

private BarChart() {
// Prevent instantiation
Expand Down Expand Up @@ -94,14 +95,18 @@ public int getHeight() {
return this.height;
}

public String getYAxisLabel() {
return this.yAxisLabel;
}

public static Builder newBarChart(String id) {
return new Builder(id);
}

@Override
public String toString() {
String pattern = "{0} '{'id: {1}, descriptionId: {2}, label: {3}, kind: {4}, width: {5}, height: {6}'}'";
return MessageFormat.format(pattern, this.getClass().getSimpleName(), this.id, this.descriptionId, this.label, this.kind, this.width, this.height);
String pattern = "{0} '{'id: {1}, descriptionId: {2}, label: {3}, kind: {4}, width: {5}, height: {6}, yAxisLabel:{7}'}'";
return MessageFormat.format(pattern, this.getClass().getSimpleName(), this.id, this.descriptionId, this.label, this.kind, this.width, this.height, this.yAxisLabel);
}

/**
Expand Down Expand Up @@ -130,6 +135,8 @@ public static final class Builder {

private int height;

private String yAxisLabel;

public Builder(String id) {
this.id = Objects.requireNonNull(id);
}
Expand Down Expand Up @@ -169,6 +176,11 @@ public Builder height(int height) {
return this;
}

public Builder yAxisLabel(String yAxisLabel) {
this.yAxisLabel = Objects.requireNonNull(yAxisLabel);
return this;
}

public BarChart build() {
BarChart barChart = new BarChart();
barChart.id = Objects.requireNonNull(this.id);
Expand All @@ -177,9 +189,10 @@ public BarChart build() {
barChart.kind = Objects.requireNonNull(this.kind);
barChart.targetObjectId = Objects.requireNonNull(this.targetObjectId);
barChart.entries = Objects.requireNonNull(this.entries);
barChart.style = this.style;
barChart.style = this.style; // Optional on purpose
barChart.width = this.width;
barChart.height = this.height;
barChart.yAxisLabel = this.yAxisLabel; // Optional on purpose
return barChart;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ public Element render() {
String targetObjectId = barChartDescription.getTargetObjectIdProvider().apply(variableManager);
List<Number> values = barChartDescription.getValuesProvider().apply(variableManager);
List<String> keys = barChartDescription.getKeysProvider().apply(variableManager);
String yAxisLabel = barChartDescription.getYAxisLabelProvider().apply(variableManager);
BarChartStyle barChartStyle = barChartDescription.getStyleProvider().apply(variableManager);

// @formatter:off
Builder builder = BarChartElementProps.newBarChartElementProps(id)
.label(label)
.descriptionId(barChartDescription.getId())
Expand All @@ -60,11 +60,13 @@ public Element render() {
.width(barChartDescription.getWidth())
.height(barChartDescription.getHeight())
.keys(keys);
// @formatter:on

if (barChartStyle != null) {
builder.style(barChartStyle);
}
if (yAxisLabel != null) {
builder.yAxisLabel(yAxisLabel);
}
BarChartElementProps barChartElementProps = builder.build();
return new Element(BarChartElementProps.TYPE, barChartElementProps);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public final class BarChartDescription implements IChartDescription {

private int height;

private Function<VariableManager, String> yAxisLabelProvider;

private BarChartDescription() {
// prevent instantiation
}
Expand Down Expand Up @@ -89,6 +91,10 @@ public int getHeight() {
return this.height;
}

public Function<VariableManager, String> getYAxisLabelProvider() {
return this.yAxisLabelProvider;
}

public static Builder newBarChartDescription(String id) {
return new Builder(id);
}
Expand Down Expand Up @@ -118,6 +124,8 @@ public static final class Builder {

private int height;

private Function<VariableManager, String> yAxisLabelProvider = variableManager -> null;

public Builder(String id) {
this.id = Objects.requireNonNull(id);
}
Expand Down Expand Up @@ -162,6 +170,11 @@ public Builder height(int height) {
return this;
}

public Builder yAxisLabelProvider(Function<VariableManager, String> yAxisLabelProvider) {
this.yAxisLabelProvider = Objects.requireNonNull(yAxisLabelProvider);
return this;
}

public BarChartDescription build() {
BarChartDescription barChartDescription = new BarChartDescription();
barChartDescription.id = Objects.requireNonNull(this.id);
Expand All @@ -173,6 +186,7 @@ public BarChartDescription build() {
barChartDescription.styleProvider = Objects.requireNonNull(this.styleProvider);
barChartDescription.width = this.width;
barChartDescription.height = this.height;
barChartDescription.yAxisLabelProvider = Objects.requireNonNull(this.yAxisLabelProvider);
return barChartDescription;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public final class BarChartElementProps implements IProps {

private int height;

private String yAxisLabel;

private BarChartElementProps() {
// prevent instantiation
}
Expand Down Expand Up @@ -87,6 +89,10 @@ public int getHeight() {
return this.height;
}

public String getYAxisLabel() {
return this.yAxisLabel;
}

public static Builder newBarChartElementProps(String id) {
return new Builder(id);
}
Expand Down Expand Up @@ -116,6 +122,8 @@ public static final class Builder {

private int height;

private String yAxisLabel;

public Builder(String id) {
this.id = Objects.requireNonNull(id);
}
Expand Down Expand Up @@ -160,6 +168,11 @@ public Builder height(int height) {
return this;
}

public Builder yAxisLabel(String yAxisLabel) {
this.yAxisLabel = Objects.requireNonNull(yAxisLabel);
return this;
}

public BarChartElementProps build() {
BarChartElementProps barChartElementProps = new BarChartElementProps();
barChartElementProps.id = Objects.requireNonNull(this.id);
Expand All @@ -171,6 +184,7 @@ public BarChartElementProps build() {
barChartElementProps.style = this.style; // Optional on purpose
barChartElementProps.width = this.width;
barChartElementProps.height = this.height;
barChartElementProps.yAxisLabel = this.yAxisLabel; // Optional on purpose
return barChartElementProps;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type BarChart implements Representation {
style: BarChartStyle
width: Int!
height: Int!
yAxisLabel: String
}

type BarChartEntry {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ export const BarChart = ({ chart }: BarChartProps) => {
const yRange = [height - marginBottom, marginTop]; // [bottom, top]

if (d3Container.current && chart) {
const {
entries: data,
metadata: { label: yLabel },
style,
} = chart;
const { entries: data, yAxisLabel, style } = chart;
const fontSize = getFontSize(style);
const fontStyle = getFontStyle(style);
const fontWeight = getFontWeight(style);
Expand Down Expand Up @@ -98,7 +94,7 @@ export const BarChart = ({ chart }: BarChartProps) => {
.attr('y', 10)
.attr('fill', 'currentColor')
.attr('text-anchor', 'start')
.text(yLabel)
.text(yAxisLabel)
);

const bar = svg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,13 @@ export interface BarChartProps {
}

export interface BarChartRepresentation {
metadata: RepresentationMetadata;
yAxisLabel: string | null;
entries: BarChartRepresentationEntry[];
style: BarChartStyle | null;
width: number;
height: number;
}

export interface RepresentationMetadata {
label: string;
}

export interface BarChartRepresentationEntry {
key: string;
value: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,6 @@ public AbstractWidgetDescription caseBarChartDescription(org.eclipse.sirius.comp
return new BarChartStyleProvider(style).build();
};

// @formatter:off
IChartDescription chartDescription = BarChartDescription.newBarChartDescription(UUID.randomUUID().toString())
.label(Optional.ofNullable(viewBarChartDescription.getName()).orElse(""))
.labelProvider(vm -> this.getWidgetLabel(viewBarChartDescription, "BarChart"))
Expand All @@ -573,8 +572,9 @@ public AbstractWidgetDescription caseBarChartDescription(org.eclipse.sirius.comp
.styleProvider(styleProvider)
.width(viewBarChartDescription.getWidth())
.height(viewBarChartDescription.getHeight())
.yAxisLabelProvider(vm -> this.getYAxisLabel(viewBarChartDescription, "Y Axis Label"))
.build();
// @formatter:on

return this.createChartWidgetDescription(viewBarChartDescription, chartDescription);
}

Expand All @@ -588,15 +588,14 @@ public AbstractWidgetDescription casePieChartDescription(org.eclipse.sirius.comp
return new PieChartStyleProvider(style).build();
};

// @formatter:off
IChartDescription chartDescription = PieChartDescription.newPieChartDescription(UUID.randomUUID().toString())
.label(this.getWidgetLabel(viewPieChartDescription, "PieChart"))
.targetObjectIdProvider(vm -> vm.get(IEditingContext.EDITING_CONTEXT, IEditingContext.class).map(IEditingContext::getId).orElse(null))
.keysProvider(vm -> List.of())
.valuesProvider(vm -> List.of())
.styleProvider(styleProvider)
.build();
// @formatter:on

return this.createChartWidgetDescription(viewPieChartDescription, chartDescription);
}

Expand Down Expand Up @@ -763,4 +762,15 @@ public String getWidgetHelpText(org.eclipse.sirius.components.view.form.WidgetDe
return helpText;
}

public String getYAxisLabel(org.eclipse.sirius.components.view.form.BarChartDescription barChartDescription, String defaultLabel) {
String yAxisLabel = defaultLabel;
String name = barChartDescription.getName();
String labelExpression = barChartDescription.getYAxisLabelExpression();
if (labelExpression != null && !labelExpression.isBlank() && !labelExpression.startsWith(AQL_PREFIX)) {
yAxisLabel = labelExpression;
} else if (name != null && !name.isBlank()) {
yAxisLabel = name;
}
return yAxisLabel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ export const BarChartWidget = ({ widget }: BarChartWidgetProps) => {
const barChartWidget = widget.chart as GQLBarChart;

const chart: BarChartRepresentation = {
metadata: {
label: 'Frequency',
},
entries: [
{ key: 'A', value: 0.08167 },
{ key: 'B', value: 0.01492 },
Expand Down Expand Up @@ -72,6 +69,7 @@ export const BarChartWidget = ({ widget }: BarChartWidgetProps) => {
style: barChartWidget.style,
width: barChartWidget.width,
height: barChartWidget.height,
yAxisLabel: 'Frequency',
};
const [selected, setSelected] = useState<boolean>(false);
const { selection } = useSelection();
Expand All @@ -89,7 +87,7 @@ export const BarChartWidget = ({ widget }: BarChartWidgetProps) => {

return (
<div
data-testid={barChartWidget.metadata.label}
data-testid={widget.label}
onFocus={() => setSelected(true)}
onBlur={() => setSelected(false)}
ref={ref}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { useSelection } from '@eclipse-sirius/sirius-components-core';
import { GQLPieChart } from '@eclipse-sirius/sirius-components-forms';
import HelpOutlineOutlined from '@mui/icons-material/HelpOutlineOutlined';
import Typography from '@mui/material/Typography';
import { makeStyles } from 'tss-react/mui';
import { useEffect, useRef, useState } from 'react';
import { makeStyles } from 'tss-react/mui';
import { PieChartWidgetProps } from './WidgetEntry.types';

const useStyles = makeStyles()((theme) => ({
Expand Down Expand Up @@ -72,14 +72,14 @@ export const PieChartWidget = ({ widget }: PieChartWidgetProps) => {

return (
<div
data-testid={pieChartWidget.metadata.label}
data-testid={widget.label}
onFocus={() => setSelected(true)}
onBlur={() => setSelected(false)}
ref={ref}
tabIndex={0}>
<div className={classes.propertySectionLabel}>
<Typography variant="subtitle2" className={selected ? classes.selected : ''}>
{pieChartWidget.metadata.label}
{widget.label}
</Typography>
{widget.hasHelpText ? <HelpOutlineOutlined color="secondary" style={{ marginLeft: 8, fontSize: 16 }} /> : null}
</div>
Expand Down
Loading

0 comments on commit 8ae9424

Please sign in to comment.