Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions src/components/chart/chart.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ $default-item-color: var(--chart-item-color, rgb(var(--contrast-1100), 0.8));
isolation: isolate;

display: flex;
flex-direction: column;
align-items: center;
gap: 0.5rem;
width: 100%;
height: 100%;
min-width: 0;
Expand Down Expand Up @@ -59,6 +62,13 @@ table {
}
}

.column-title {
@include mixins.truncate-text;
position: absolute;
top: -1.5rem;
max-width: 100%;
}

*,
*:before,
*:after {
Expand Down
45 changes: 39 additions & 6 deletions src/components/chart/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* @exampleComponent limel-example-chart-orientation
* @exampleComponent limel-example-chart-max-value
* @exampleComponent limel-example-chart-type-bar
* @exampleComponent limel-example-chart-column-titles
* @exampleComponent limel-example-chart-type-dot
* @exampleComponent limel-example-chart-type-area
* @exampleComponent limel-example-chart-type-line
Expand Down Expand Up @@ -111,6 +112,19 @@
@Prop({ reflect: true })
public loading: boolean = false;

/**
* When set to `true`, displays column titles for bar charts.
* By default, column titles are not rendered.
*/
@Prop({ reflect: true })
public showColumnTitles: boolean = false;
Comment on lines +115 to +120
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Clarify that showColumnTitles applies to landscape bar charts

The implementation only shows column titles when type === 'bar' and orientation === 'landscape', but the doc comment currently mentions only “bar charts”. Consider making that explicit so consumers understand why nothing changes for portrait bar charts.

For example:

/**
 * When set to `true`, displays column titles for bar charts
 * in landscape orientation. By default, column titles are not rendered.
 */
@Prop({ reflect: true })
public showColumnTitles: boolean = false;
🤖 Prompt for AI Agents
In src/components/chart/chart.tsx around lines 115 to 120, the Prop doc comment
for showColumnTitles is misleading because it implies it applies to all bar
charts but the implementation only renders column titles when type === 'bar' and
orientation === 'landscape'; update the JSDoc to explicitly state that the flag
controls column titles for bar charts in landscape orientation (and that
portrait bar charts are unaffected) so consumers understand the scope of the
prop.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is actually a red flag. Let's think a bit more about how we should solve this 🤔

  1. We shouldn't have a prop that only sometimes works. Preferably, we should cover most of the types if possible. Can we somehow display it for others too?
  2. If this is only for bar chart, then
  • it should work for both landscape and portrait modes.
  • the name of the prop should be more descriptive


/**
* Label for the horizontal axis (X-axis).
*/
@Prop({ reflect: true })
public xAxisLabel?: string;
Comment on lines +122 to +126
Copy link
Copy Markdown
Contributor

@Kiarokh Kiarokh Dec 1, 2025

Choose a reason for hiding this comment

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

This one is a new separate feature. It should not be committed together with the other one 😊

It also has the same problem like the previous one. The chart component has various modes or types. Since all of them are 2D, the x-axis is applicable to all. Basically, all of them have an x-axis. But in some, like the circular ones, the meaning of x-axis would be really strange.

Further down, the this prop is implemented in a way where it will always show up. Even under a pie or doughnut chart.

Additionally, there is already a prop called accessibleItemsLabel which is basically what you are trying to do.

    /**
     * Helps users of assistive technologies to understand
     * what the items in the chart represent.
     */
    @Prop({ reflect: true })
    public accessibleItemsLabel?: string;

Further,
On landscape orientation, the x-axis is below. But on it can be on the left side on the portrait mode. So this name becomes confusing


private range: {
minValue: number;
maxValue: number;
Expand All @@ -132,21 +146,30 @@
return <limel-spinner limeBranded={false} />;
}

return (
return [
<table
aria-busy={this.loading ? 'true' : 'false'}
aria-live="polite"
style={{
'--limel-chart-number-of-items':
this.items.length.toString(),
}}
>
{this.renderCaption()}
{this.renderTableHeader()}
{this.renderAxises()}
<tbody class="chart">{this.renderItems()}</tbody>
</table>
);
</table>,

Check warning on line 162 in src/components/chart/chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Missing "key" prop for element in array

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZrJnYcH_BhtGh_eOdWs&open=AZrJnYcH_BhtGh_eOdWs&pullRequest=3742
this.renderXAxisLabel(),
];
}

private renderXAxisLabel() {
if (!this.xAxisLabel) {
return;
}

return <div>{this.xAxisLabel}</div>;
}

private renderCaption() {
Expand Down Expand Up @@ -237,7 +260,7 @@
>
<th>{this.getItemText(item)}</th>
<td>{this.getFormattedValue(item)}</td>
{this.renderTooltip(item, itemId, size)}
{this.renderItemLabel(item, itemId, size)}
</tr>
);
});
Expand Down Expand Up @@ -322,7 +345,7 @@
return item.text;
}

private renderTooltip(item: ChartItem, itemId: string, size: number) {
private renderItemLabel(item: ChartItem, itemId: string, size: number) {
const text = this.getItemText(item);
const PERCENT_DECIMAL = 2;
const formattedValue = this.getFormattedValue(item);
Expand All @@ -337,14 +360,24 @@
tooltipProps.label = `${text} (${size.toFixed(PERCENT_DECIMAL)}%)`;
}

return (
const tooltip = (
<limel-tooltip
{...tooltipProps}
openDirection={
this.orientation === 'portrait' ? 'right' : 'top'
}
/>
);

if (
this.showColumnTitles &&
this.orientation === 'landscape' &&
this.type === 'bar'
) {
return [<div class="column-title">{text}</div>, tooltip];

Check warning on line 377 in src/components/chart/chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Missing "key" prop for element in array

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZrF8hvs7q8XMp_X1pHS&open=AZrF8hvs7q8XMp_X1pHS&pullRequest=3742
}

return tooltip;
}

private calculateRange() {
Expand Down
50 changes: 50 additions & 0 deletions src/components/chart/examples/chart-column-titles.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { Component, h, Host, State } from '@stencil/core';
import { chartItems } from './chart-items-bar';

/**
* Bar chart with column titles
*
* By setting the `showColumnTitles` prop to `true`, you can display column titles
* for bar charts in landscape orientation. This is useful when you want to clearly
* label each bar with its category name directly in the chart, rather than relying
* on tooltips.
*
* When `showColumnTitles` is `false` (the default), the chart will use tooltips
* to display item information on hover instead.
*/
@Component({
tag: 'limel-example-chart-column-titles',
shadow: true,
styleUrl: 'chart-examples.scss',
})
export class ChartColumnTitlesExample {
@State()
private showColumnTitles = true;

public render() {
return (
<Host class="large">
<h4>Monthly Sales Performance</h4>
<limel-chart
type="bar"
items={chartItems}
orientation="landscape"
showColumnTitles={this.showColumnTitles}
xAxisLabel="Months"
maxValue={100}
/>
<limel-example-controls>
<limel-switch
label="Show column titles"
value={this.showColumnTitles}
onChange={this.handleToggleChange}
/>
</limel-example-controls>
</Host>
);
}

private handleToggleChange = (event: CustomEvent<boolean>) => {

Check warning on line 47 in src/components/chart/examples/chart-column-titles.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Member 'handleToggleChange' is never reassigned; mark it as `readonly`.

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZrF8hs_7q8XMp_X1pHR&open=AZrF8hs_7q8XMp_X1pHR&pullRequest=3742
this.showColumnTitles = event.detail;
};
Comment on lines +47 to +49
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Mark handleToggleChange as readonly

The member is never reassigned and is used as an event handler; marking it readonly matches the pattern used in chart.tsx and satisfies the static analysis suggestion.

-export class ChartColumnTitlesExample {
+export class ChartColumnTitlesExample {
@@
-    private handleToggleChange = (event: CustomEvent<boolean>) => {
+    private readonly handleToggleChange = (event: CustomEvent<boolean>) => {
         this.showColumnTitles = event.detail;
     };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private handleToggleChange = (event: CustomEvent<boolean>) => {
this.showColumnTitles = event.detail;
};
private readonly handleToggleChange = (event: CustomEvent<boolean>) => {
this.showColumnTitles = event.detail;
};
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis

[warning] 46-46: Member 'handleToggleChange' is never reassigned; mark it as readonly.

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZrF8hs_7q8XMp_X1pHR&open=AZrF8hs_7q8XMp_X1pHR&pullRequest=3742

🤖 Prompt for AI Agents
In src/components/chart/examples/chart-column-titles.tsx around lines 46 to 48,
the event handler handleToggleChange is never reassigned but is defined as a
mutable class field; change its declaration to a readonly class field (e.g. add
the readonly modifier to "private readonly handleToggleChange = ...") so it
matches the pattern used in chart.tsx and satisfies the static analysis
suggestion while keeping the same arrow-function/event-handler behavior.

}
1 change: 1 addition & 0 deletions src/components/chart/examples/chart-type-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export class ChartTypeBarExample {
items={chartItems}
orientation={this.orientation}
maxValue={this.maxValue}
showColumnTitles={true}
/>
<limel-example-controls>
<limel-select
Expand Down
1 change: 1 addition & 0 deletions src/components/chart/partial-styles/_bar-gantt-dot.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
}

.item {
position: relative;
display: flex;
align-items: center;
justify-content: center;
Expand Down
Loading