Skip to content

Conversation

@vursen
Copy link
Contributor

@vursen vursen commented Jun 21, 2021

Description

LitElement reserves the update() method so that we cannot use it to update chart configuration anymore.

This PR deprecates update() and introduces updateConfiguration() instead. Though calling update() still updates chart configuration, now it shows a deprecation warning as well.

Part of #73 (issue)

Type of change

  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs-beta/latest/guide/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.

@vursen vursen added the no-polymer Removing Polymer from Vaadin public APIs label Jun 21, 2021
@vursen vursen self-assigned this Jun 21, 2021
chart.setAttribute('category-min', newCategoryMin);
});

it('should warn when setting a not valid minimum value', () => {
Copy link
Contributor Author

@vursen vursen Jun 21, 2021

Choose a reason for hiding this comment

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

It was an attempt to improve the chart's code coverage, but it didn't significantly change the numbers (+0.25%). I decided not to throw away the created test from the PR anyway.

chart.setAttribute('category-max', newCategoryMax);
});

it('should warn when setting a not valid maximum value', () => {
Copy link
Contributor Author

@vursen vursen Jun 21, 2021

Choose a reason for hiding this comment

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

It was an attempt to improve the chart's code coverage, but it didn't significantly change the situation (+0.25%). I decided not to throw away the created test from the PR anyway.

@vursen
Copy link
Contributor Author

vursen commented Jun 21, 2021

All the tests pass, there is only the problem with the chart's code coverage which in fact is well-known and not associated with the PR:

Coverage for branches failed with 46.93 % compared to configured 50 %

@vursen vursen requested a review from tomivirkki June 21, 2021 14:51
@vursen vursen requested a review from tomivirkki June 22, 2021 09:59
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vursen vursen merged commit 635c9fe into master Jun 22, 2021
@vursen vursen deleted the feat/deprecate-update branch June 22, 2021 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-polymer Removing Polymer from Vaadin public APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants