pr-explainer-bot[bot]
commented
1 year ago Pull Request Review - Summary
Hey there! 👋 Here's a summary of the previous tasks and their results for the pull request review. Let's dive in!
Changes
- In
task1, import statements were changed in various files to improve code organization and maintain consistency. - In
task2, suggestions were made to improve code readability and consistency at specific lines. - In
task3, potential bugs were identified in specific files and recommendations were provided. - In
task4, suggestions were made to refactor code for better readability in a specific file.
Suggestions
- In
task2, specific lines were pointed out where improvements can be made, such as removing unnecessary empty lines and adding helpful comments.
Bugs
- In
task3, specific files were identified with missing newlines and semicolons, which should be added to fix the issues.
Improvements
- In
task4, a specific method in theChart.javafile was suggested to be refactored for better readability.
Rating
Unfortunately, no rating was provided for the code in task5. It would have been helpful to have a rating on a scale of 0 to 10, considering criteria like readability, performance, and security.
That's it for the summary! If you have any further questions or need more details, feel free to ask. And don't forget, we also offer a premium plan that can handle bigger pull requests with more context. 😉
Happy coding!
ennerf
codecov[bot]
sonarcloud[bot]
Catch issues before they fail your Quality Gate with our IDE extension 
Before
The styling was previously based on Strings that could be applied to datasets as well as to individual data points. Some reasons for doing it this way were that the style needs to be storable and usable without a JavaFX dependency (DataSets are not dependent on JavaFX).
Problems
=, which is not valid CSSAfter
Each renderer is now part of the SceneGraph and generates a styleable node for each dataset (a dataset can be in multiple renderers, but only once per renderer). The nodes get styled with JavaFX properties and apply the source
dataSet.getStyle(), which was changed to match the JavaFX properties. The individual styles still use strings, but there is now no more overhead when they are not used.hatchShiftByIndexnow also applies without custom styles, so multiple datasets can show error surfaces simultaneouslyWays to style DataSets
1) directly on the Node
Quick way to style individual datasets
2) via CSS and classes
Good way to style certain classes of data that should always look the same
3) via style strings and a utility class
Other Behavior changes
getDataSetsfrom the Chart. The method is now a utility method that returns the datasets of the first renderer.Bugs and things that need to be looked at after this PR
Chart::getDataSets, so I'm not sure what to do thereMore styling that didn't fit into this PR
The PR is already very large as is, so I didn't want to pack in more. However, after merging there are a few more styling things that I'd like to take a look at:
improve the CSS color lookup and add a theme class to support Modena-based and AtlantaFX-based themes (e.g. darkmode) out of the box
[x] go over the used colors again and make sure they make sense. Instead of using
-fx-strokefor lines and markers, maybe it makes sense to have dedicated-fx-line-colorand-fx-marker-colorproperties[x] some of the renderer properties should probably be moved to the dataset nodes (e.g. marker size)
There are many classes that aren't used much anymore (e.g. Events) that should get removed once they are removed from the examples