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

feat: add Gantt specific API to Charts #6997

Merged
merged 14 commits into from
Jan 15, 2025
Merged

Conversation

bwajtr
Copy link
Contributor

@bwajtr bwajtr commented Jan 3, 2025

Description

This change brings support for Gantt specific API to Vaadin Charts.

A new ChartType has been introduced to be able to activate Gantt chart feature.
Also, after discussion with Diego and Rolf we:

  • decided to support only GanttSeries with Gantt charts (so although Highcharts theoretically supports also XRange series in the Gantt chart, we will not support this case)
  • decided to make a combination of Gantt chart and timeline property invalid - in the web component the value of timeline is ignored when chart is Gantt chart, in the Java api we throw IllegalArgumentException when somebody tries to use the Gantt chart and activate timeline at the same time.

New classes or attributes were added to cover the Highcharts 9.2.2 Gantt Chart API. Changes to existing API classes reflect probably the changes between earlier versions of Highcharts and the currently supported 9.2.2. In any case, if change to existing class was made, I made sure it's valid also for other Highcharts charts.

I've added as many properties as I could, hopefully not missing anything - with the exception of "drag and drop" support (series.gantt.dragDrop)... however, that is not supported anywhere in the Vaadin Charts API.

I've added a lot of Demos in form of Integration tests to verify that the changes in the API work.

Fixes #4731

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.
  • I have not completed some of the steps above and my pull request can be closed immediately.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

@bwajtr bwajtr changed the title feat: improve charts to support Gantt specific API feat: add Gantt specific API to Charts Jan 9, 2025
@bwajtr
Copy link
Contributor Author

bwajtr commented Jan 9, 2025

@DiegoCardoso is there a way how to get around th SonarCloud Code Analysis? It's bragging about the code duplication (even in the existing Chart files) - but to be honest the Chart API is full of duplication and I really don't feel like I should fix that in scope of this task

@DiegoCardoso
Copy link
Contributor

@DiegoCardoso is there a way how to get around th SonarCloud Code Analysis? It's bragging about the code duplication (even in the existing Chart files) - but to be honest the Chart API is full of duplication and I really don't feel like I should fix that in scope of this task

There are some battles we can't really win. I think we can ignore them. The SonarCloud Code Analysis is not a required check.

Copy link
Contributor

@DiegoCardoso DiegoCardoso left a comment

Choose a reason for hiding this comment

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

Added a few comments to the PR.

@bwajtr bwajtr requested a review from DiegoCardoso January 15, 2025 07:50
@bwajtr bwajtr enabled auto-merge (squash) January 15, 2025 09:31
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
32.1% Duplication on New Code (required ≤ 10%)

See analysis details on SonarQube Cloud

@bwajtr bwajtr merged commit 091e4a0 into main Jan 15, 2025
4 of 5 checks passed
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.7.0.alpha5 and is also targeting the upcoming stable 24.7.0 version.

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

Successfully merging this pull request may close these issues.

Charts: Support for gantt charts
3 participants