-
Notifications
You must be signed in to change notification settings - Fork 95
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
Axis and renderer CSS Styling #598
Conversation
Pull Request Review MarkdownHey there! 👋 Here's a summary of the changes, suggestions, bugs, improvements, and rating for the pull request. Let's dive in! Changes
Suggestions
Bugs
Improvements
@Override
protected CssPropertyFactory RatingOverall, the code changes look good, with some minor suggestions and a couple of potential bugs. I would rate the code a 7 out of 10 based on readability, performance, and security. That's it for the pull request review! Let me know if you need any further assistance. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work, looks good and can be merged once the tests are fixed.
chartfx-chart/src/main/java/io/fair_acc/chartfx/axes/spi/AbstractAxis.java
Show resolved
Hide resolved
SonarCloud Quality Gate failed. 11 Bugs No Coverage information Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #598 +/- ##
============================================
- Coverage 50.27% 49.71% -0.56%
+ Complexity 6268 6246 -22
============================================
Files 375 379 +4
Lines 37531 37625 +94
Branches 6143 6165 +22
============================================
- Hits 18867 18707 -160
- Misses 17414 17687 +273
+ Partials 1250 1231 -19
☔ View full report in Codecov by Sentry. |
This PR includes some styling changes for the components that are comparatively easy to deal with. Datasets will require more work and will be in a separate PR.
->
getLabel.setFill(color)`. This may result in a bit more effort to port, but it cleans up the API and should help encourage the use of CSS.vertical
,horizontal
,center
)Bugfixes
Demos with Sass and live CSS reloading
Notes
NumericAxis
) gets initialized with the static properties from the parent class before they are valid. This is a bug in current main.