-
Notifications
You must be signed in to change notification settings - Fork 80
[5.1] docs(charts): update trendline articles to include the new types #1887
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ikoevska
reviewed
Jan 26, 2024
Contributor
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.
General notes from the review:
- You don't really need the monospace formatting in the bulleted list at the beginning of the page. In fact, you can drop it from all instances. It makes sense to use monospace formatting if you're using the correct spelling used in the code.
- I took the liberty of suggesting a "Use X to achieve Y" template for the descriptions of the charts. It's more action-oriented and provides clear goals at a glance. Feel free to rework the suggestions as needed, if I messed up the accuracy.
- Personally, I wonder if Supported Series should be moved to the beginning of the page. So that if you're working with something that's not supported, you could safely skip the page.
- This one seems out of the scope of these changes, but I would have loved to see a more detailed explanation about how to configure/implement trendlines (I know it's obvious in the code, but ideally, there would be a section/a paragraph/one line saying where and when in your code it's appropriate to implement it, possibly, a general template or any extra info needed to make it work, what are some best practices for Blazor trendlines (if any), tips and tricks (or links to them, possibly links to applicable KBs), styling.
dimodi
reviewed
Jan 26, 2024
ikoevska
approved these changes
Jan 29, 2024
Contributor
ikoevska
left a comment
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.
🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to: https://github.com/telerik/blazor/issues/7966.