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.

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

[Horizontal Bar Chart With Axis] Enable multiple legend selection #33484

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

srmukher
Copy link
Contributor

[Horizontal Bar Chart With Axis] Enable multiple legend selection

@srmukher srmukher requested a review from a team as a code owner December 18, 2024 08:01
Copy link

github-actions bot commented Dec 18, 2024

📊 Bundle size report

✅ No changes found

Copy link

Pull request demo site: URL

@@ -54,6 +54,7 @@ export interface IHorizontalBarChartWithAxisState extends IBasestate {
callOutAccessibilityData?: IAccessibilityProps;
Copy link
Collaborator

@fabricteam fabricteam Dec 18, 2024

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual regressions to review in the fluentuiv8 Visual Regression Report

Callout 6 screenshots
Image Name Diff(in Pixels) Image Type
Callout.Right bottom edge.chromium.png 3005 Changed
Callout.Bottom Right Edge RTL.chromium.png 1593 Changed
Callout.Bottom auto edge.chromium.png 2309 Changed
Callout.No callout width specified.chromium.png 2319 Changed
Callout.Left center.chromium.png 2616 Changed
Callout.No beak.chromium.png 2306 Changed
react-charting-AreaChart 1 screenshots
Image Name Diff(in Pixels) Image Type
react-charting-AreaChart.Custom Accessibility.chromium.png 11 Changed
react-charting-LineChart 1 screenshots
Image Name Diff(in Pixels) Image Type
react-charting-LineChart.Gaps.chromium.png 1 Changed

@srmukher srmukher force-pushed the users/srmukher/HBC_multi_legend branch from 403da6d to 5d4b991 Compare December 19, 2024 06:10
if (this._isLegendSelected()) {
if (this._isSpecificLegendTitleSelected(currentLegend?.title!)) {
this.setState({
isLegendSelected: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

isLegendSelected: false

why setting this to false if setting selectedLegendTitle to a value

@@ -780,11 +760,61 @@ export class HorizontalBarChartWithAxisBase extends React.Component<
focusZonePropsInHoverCard={this.props.focusZonePropsForLegendsInHoverCard}
overflowText={this.props.legendsOverflowText}
{...this.props.legendProps}
canSelectMultipleLegends={this.props.legendProps?.canSelectMultipleLegends}
Copy link
Contributor

Choose a reason for hiding this comment

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

canSelectMultipleLegends={this.props.legendProps?.canSelectMultipleLegends}

this line is not needed

this.state.isLegendSelected === false ||
(this.state.isLegendSelected && this.state.selectedLegendTitle === point.legend)
) {
if (!this._isLegendSelected() || (this._isLegendSelected() && this._isSpecificLegendTitleSelected(point.legend!))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

(!this._isLegendSelected() || (this._isLegendSelected() && this._isSpecificLegendTitleSelected(point.legend!))) {

shouldn't bar hover and bar focus conditions be same?

shouldHighlight = this.state.selectedLegendTitle === point.legend;
if (this._isLegendHovered() || this._isLegendSelected()) {
shouldHighlight =
this._isSpecificLegendTitleSelected(point.legend!) || this._isSpecificLegendSelected(point.legend!);
Copy link
Contributor

Choose a reason for hiding this comment

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

this._isSpecificLegendTitleSelected(point.legend!) || this._isSpecificLegendSelected(point.legend!);

same here?

if (this.state.isLegendHovered || this.state.isLegendSelected) {
shouldHighlight = this.state.selectedLegendTitle === point.legend;
if (this._isLegendHovered() || this._isLegendSelected()) {
shouldHighlight = this._isSpecificLegendTitleSelected(point.legend!);
Copy link
Contributor

Choose a reason for hiding this comment

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

and here?

@AtishayMsft
Copy link
Contributor

Change file is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants