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

display: flex;
flex-direction: column;
gap: 0.5rem;
width: 100%;
height: 100%;
min-width: 0;
Expand Down Expand Up @@ -104,6 +106,50 @@ limel-spinner {
margin: auto;
}

.legend {
display: flex;
align-self: center;
max-width: 80%;
font-size: small;

background-color: rgb(var(--contrast-500));
border-radius: 0.5rem;
padding: 0.5rem;

.legend-content {
display: flex;
width: 100%;
overflow-x: auto;
gap: 1rem;

@include mixins.fade-out-overflowed-content-on-edges(horizontally);
}
}

.legend-item {
display: flex;
align-items: center;
gap: 0.25rem;
cursor: pointer;
user-select: none;
white-space: nowrap; // Prevent text wrapping
flex-shrink: 0; // Prevent items from shrinking

&--hidden {
opacity: 0.5;
}

limel-badge {
transition:
opacity 0.2s,
filter 0.2s;
}

&:hover limel-badge {
filter: brightness(1.2);
}
}
Comment on lines 109 to 177
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Avoid BEM-style class names in shadow-DOM components.

The .legend-item--hidden uses BEM-style naming, which is unnecessary since this component uses shadow-DOM for style scoping.

As per coding guidelines.

Apply this diff to simplify the class naming:

-.legend-item {
+.legend-item {
     display: flex;
     align-items: center;
     gap: 0.25rem;
     cursor: pointer;
     user-select: none;
     white-space: nowrap; // Prevent text wrapping
     flex-shrink: 0; // Prevent items from shrinking
 
-    &--hidden {
+    &.hidden {
         opacity: 0.5;
     }
 
     limel-badge {
         transition:
             opacity 0.2s,
             filter 0.2s;
     }
 
     &:hover limel-badge {
         filter: brightness(1.2);
     }
 }

Then update the corresponding class usage in src/components/chart/chart.tsx at line 277:

                             class={{
                                 'legend-item': true,
-                                'legend-item--hidden': isHidden,
+                                'hidden': isHidden,
                             }}
📝 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
.legend {
display: flex;
align-self: center;
max-width: 80%;
font-size: small;
background-color: rgb(var(--contrast-500));
border-radius: 0.5rem;
padding: 0.5rem;
.legend-content {
display: flex;
width: 100%;
overflow-x: auto;
gap: 1rem;
@include mixins.fade-out-overflowed-content-on-edges(horizontally);
}
}
.legend-item {
display: flex;
align-items: center;
gap: 0.25rem;
cursor: pointer;
user-select: none;
white-space: nowrap; // Prevent text wrapping
flex-shrink: 0; // Prevent items from shrinking
&--hidden {
opacity: 0.5;
}
limel-badge {
transition:
opacity 0.2s,
filter 0.2s;
}
&:hover limel-badge {
filter: brightness(1.2);
}
}
.legend {
display: flex;
align-self: center;
max-width: 80%;
font-size: small;
background-color: rgb(var(--contrast-500));
border-radius: 0.5rem;
padding: 0.5rem;
.legend-content {
display: flex;
width: 100%;
overflow-x: auto;
gap: 1rem;
@include mixins.fade-out-overflowed-content-on-edges(horizontally);
}
}
.legend-item {
display: flex;
align-items: center;
gap: 0.25rem;
cursor: pointer;
user-select: none;
white-space: nowrap; // Prevent text wrapping
flex-shrink: 0; // Prevent items from shrinking
&.hidden {
opacity: 0.5;
}
limel-badge {
transition:
opacity 0.2s,
filter 0.2s;
}
&:hover limel-badge {
filter: brightness(1.2);
}
}
🤖 Prompt for AI Agents
In src/components/chart/chart.scss around lines 109 to 151 and update usage in
src/components/chart/chart.tsx at line 277: replace the BEM-style modifier class
.legend-item--hidden with a simpler modifier name (e.g., .legend-item-hidden or
.hidden) in the SCSS and update the JSX to add/remove that new class name
instead of .legend-item--hidden so class names match; keep all other styles
intact and ensure hover/transition rules still target the renamed selector.


@mixin line(
$direction: vertical,
$color: rgb(var(--contrast-800), 0.4),
Expand Down
159 changes: 122 additions & 37 deletions src/components/chart/chart.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Component, Event, EventEmitter, h, Prop, Watch } from '@stencil/core';
import {
Component,
Event,
EventEmitter,
h,
Prop,
State,
Watch,
} from '@stencil/core';
import { Languages } from '../date-picker/date.types';
import translate from '../../global/translations';
import { createRandomString } from '../../util/random-string';
Expand Down Expand Up @@ -117,6 +125,9 @@ export class Chart {
totalRange: number;
};

@State()
private hiddenItems: Set<string> = new Set();

/**
* Fired when a chart item with `clickable` set to `true` is clicked
*/
Expand All @@ -132,7 +143,7 @@ export class Chart {
return <limel-spinner limeBranded={false} />;
}

return (
return [
<table
aria-busy={this.loading ? 'true' : 'false'}
aria-live="polite"
Expand All @@ -145,8 +156,9 @@ export class Chart {
{this.renderTableHeader()}
{this.renderAxises()}
<tbody class="chart">{this.renderItems()}</tbody>
</table>
);
</table>,
this.renderLegend(this.items),
];
}

private renderCaption() {
Expand Down Expand Up @@ -212,37 +224,97 @@ export class Chart {

let cumulativeOffset = 0;

return this.items.map((item, index) => {
const itemId = createRandomString();
const sizeAndOffset = this.calculateSizeAndOffset(item);
const size = sizeAndOffset.size;
let offset = sizeAndOffset.offset;

if (this.type === 'pie' || this.type === 'doughnut') {
offset = cumulativeOffset;
cumulativeOffset += size;
}

return (
<tr
style={this.getItemStyle(item, index, size, offset)}
class={this.getItemClass(item)}
key={itemId}
id={itemId}
data-index={index}
tabIndex={0}
role={item.clickable ? 'button' : null}
onClick={this.handleClick}
onKeyDown={this.handleKeyDown}
>
<th>{this.getItemText(item)}</th>
<td>{this.getFormattedValue(item)}</td>
{this.renderTooltip(item, itemId, size)}
</tr>
);
});
return this.items
.filter((item) => !this.hiddenItems.has(item.text))
.map((item, index) => {
const itemId = createRandomString();
const sizeAndOffset = this.calculateSizeAndOffset(item);
const size = sizeAndOffset.size;
let offset = sizeAndOffset.offset;

if (this.type === 'pie' || this.type === 'doughnut') {
offset = cumulativeOffset;
cumulativeOffset += size;
}

return (
<tr
style={this.getItemStyle(item, index, size, offset)}
class={this.getItemClass(item)}
key={itemId}
id={itemId}
data-index={index}
tabIndex={0}
role={item.clickable ? 'button' : null}
onClick={this.handleClick}
onKeyDown={this.handleKeyDown}
>
<th>{this.getItemText(item)}</th>
<td>{this.getFormattedValue(item)}</td>
{this.renderTooltip(item, itemId, size)}
</tr>
);
});
}

private renderLegend(items: ChartItem[]) {
// Only render legend if at least one item has a color
const hasColoredItems = items.some((item) => item.color);

if (!hasColoredItems) {
return;
}

return (
<div class="legend">
<div class="legend-content">
{items.map((item) => {
const isHidden = this.hiddenItems.has(item.text);
return (
<div
class={{
'legend-item': true,
'legend-item--hidden': isHidden,
}}
key={item.text}
onClick={() => this.handleLegendClick(item)}
>
Comment on lines +290 to +297
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add keyboard support to legend items for accessibility.

The legend item div has an onClick handler but lacks keyboard event handlers, making it inaccessible to keyboard-only users.

Apply this diff to add keyboard support:

                             <div
                                 class={{
                                     'legend-item': true,
                                     'legend-item--hidden': isHidden,
                                 }}
                                 key={item.text}
+                                tabIndex={0}
+                                role="button"
                                 onClick={() => this.handleLegendClick(item)}
+                                onKeyDown={(e) => {
+                                    if (e.key === 'Enter' || e.key === ' ') {
+                                        e.preventDefault();
+                                        this.handleLegendClick(item);
+                                    }
+                                }}
                             >
📝 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
<div
class={{
'legend-item': true,
'legend-item--hidden': isHidden,
}}
key={item.text}
onClick={() => this.handleLegendClick(item)}
>
<div
class={{
'legend-item': true,
'legend-item--hidden': isHidden,
}}
key={item.text}
tabIndex={0}
role="button"
onClick={() => this.handleLegendClick(item)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
this.handleLegendClick(item);
}
}}
>
🧰 Tools
🪛 Biome (2.1.2)

[error] 274-281: Static Elements should not be interactive.

To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.

(lint/a11y/noStaticElementInteractions)


[error] 274-281: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.

(lint/a11y/useKeyWithClickEvents)

🤖 Prompt for AI Agents
In src/components/chart/chart.tsx around lines 274 to 281, the legend item div
has an onClick but no keyboard support; make it accessible by adding keyboard
handlers and focusability: add role="button" and tabIndex={0} to make it
focusable/semantically interactive, implement an onKeyDown that listens for
Enter and Space and calls this.handleLegendClick(item) (prevent default on
Space), and include an appropriate aria attribute like aria-pressed or
aria-checked if the item represents toggle state; keep the existing onClick to
call the same handler so mouse and keyboard share behavior.

<limel-badge
style={{
'--badge-background-color': item.color,
cursor: 'pointer',
opacity: isHidden ? '0.6' : '1',
}}
/>
<span
style={{ opacity: isHidden ? '0.6' : '1' }}
>
{this.getItemText(item)}
</span>
</div>
);
})}
</div>
</div>
);
}
Comment on lines 267 to 316
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Ensure unique keys for legend items.

Using item.text as the key assumes all items have unique text values. If duplicate text values exist, this will cause key collisions and rendering issues.

Consider one of these solutions:

Solution 1: Use text with index for uniqueness

-                    {items.map((item) => {
+                    {items.map((item, index) => {
                         const isHidden = this.hiddenItems.has(item.text);
                         return (
                             <div
                                 class={{
                                     'legend-item': true,
                                     'legend-item--hidden': isHidden,
                                 }}
-                                key={item.text}
+                                key={`${item.text}-${index}`}
                                 onClick={() => this.handleLegendClick(item)}
                             >

Solution 2: Add a unique ID to ChartItem

If items should have globally unique identifiers, consider adding an optional id field to the ChartItem interface and using that as the key.

📝 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 renderLegend(items: ChartItem[]) {
// Only render legend if at least one item has a color
const hasColoredItems = items.some((item) => item.color);
if (!hasColoredItems) {
return;
}
return (
<div class="legend">
<div class="legend-content">
{items.map((item) => {
const isHidden = this.hiddenItems.has(item.text);
return (
<div
class={{
'legend-item': true,
'legend-item--hidden': isHidden,
}}
key={item.text}
onClick={() => this.handleLegendClick(item)}
>
<limel-badge
style={{
'--badge-background-color': item.color,
cursor: 'pointer',
opacity: isHidden ? '0.6' : '1',
}}
/>
<span
style={{ opacity: isHidden ? '0.6' : '1' }}
>
{this.getItemText(item)}
</span>
</div>
);
})}
</div>
</div>
);
}
private renderLegend(items: ChartItem[]) {
// Only render legend if at least one item has a color
const hasColoredItems = items.some((item) => item.color);
if (!hasColoredItems) {
return;
}
return (
<div class="legend">
<div class="legend-content">
{items.map((item, index) => {
const isHidden = this.hiddenItems.has(item.text);
return (
<div
class={{
'legend-item': true,
'legend-item--hidden': isHidden,
}}
key={`${item.text}-${index}`}
onClick={() => this.handleLegendClick(item)}
>
<limel-badge
style={{
'--badge-background-color': item.color,
cursor: 'pointer',
opacity: isHidden ? '0.6' : '1',
}}
/>
<span
style={{ opacity: isHidden ? '0.6' : '1' }}
>
{this.getItemText(item)}
</span>
</div>
);
})}
</div>
</div>
);
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 274-281: Static Elements should not be interactive.

To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.

(lint/a11y/noStaticElementInteractions)


[error] 274-281: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.

(lint/a11y/useKeyWithClickEvents)

🤖 Prompt for AI Agents
In src/components/chart/chart.tsx around lines 260 to 300, the legend uses
item.text as the React/vdom key which can collide when texts are duplicated; add
a unique key strategy: extend ChartItem with an optional id field (if
appropriate) and use that as the key when present, otherwise fall back to a
deterministic fallback such as combining item.text with the item index (e.g.
`${item.text}-${index}`) to guarantee uniqueness; update the ChartItem
type/interface and the renderLegend mapping to use the new id-or-text-with-index
key.


private handleLegendClick = (item: ChartItem) => {
const updatedHiddenItems = new Set(this.hiddenItems);

if (updatedHiddenItems.has(item.text)) {
updatedHiddenItems.delete(item.text);
} else {
updatedHiddenItems.add(item.text);
}

this.hiddenItems = updatedHiddenItems;

// Force recalculation of range when visibility changes
this.range = null;
this.recalculateRangeData();
};

private getItemStyle(
item: ChartItem,
index: number,
Expand Down Expand Up @@ -352,9 +424,21 @@ export class Chart {
return this.range;
}

const minRange = Math.min(0, ...this.items.map(this.getMinimumValue));
const maxRange = Math.max(...this.items.map(this.getMaximumValue));
const totalSum = this.items.reduce(
// Use only visible items for range calculation
const visibleItems = this.items.filter(
(item) => !this.hiddenItems.has(item.text)
);
const itemsForCalculation =
visibleItems.length > 0 ? visibleItems : this.items;

const minRange = Math.min(
0,
...itemsForCalculation.map(this.getMinimumValue)
);
const maxRange = Math.max(
...itemsForCalculation.map(this.getMaximumValue)
);
const totalSum = itemsForCalculation.reduce(
(sum, item) => sum + this.getMaximumValue(item),
0
);
Expand All @@ -368,7 +452,8 @@ export class Chart {
}

if (!this.axisIncrement) {
this.axisIncrement = this.calculateAxisIncrement(this.items);
this.axisIncrement =
this.calculateAxisIncrement(itemsForCalculation);
}

const visualMaxValue =
Expand Down