-
Notifications
You must be signed in to change notification settings - Fork 912
Add Spider Chart Component to Radzen Blazor #2278
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
base: master
Are you sure you want to change the base?
Conversation
Referencing the Feature request |
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.
Hi @ehabhussein,
Thank you for your contribution!
I have made the first review pass and left some comments that need your attention before merging. Let me know if you have any questions.
*.md | ||
/.gitignore | ||
/.gitignore | ||
/nul |
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.
Those modifications look random. Revert to the original content of the .gitignore file.
@typeparam TItem | ||
@inherits RadzenComponent | ||
|
||
<div @ref="@Element" class="@GetComponentCssClass() rz-scheme-@(ColorScheme.ToString().ToLower()) @GetCssClass()" style="@Style" @attributes="Attributes"> |
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.
Check how RadzenChart implements the CSS class and do it in the same way: https://github.com/radzenhq/radzen-blazor/blob/master/Radzen.Blazor/RadzenChart.razor.cs#L594
var radius = Math.Min(centerX, centerY) * 0.8 * (i / 5.0); | ||
<circle cx="@centerX" cy="@centerY" r="@radius" | ||
fill="none" | ||
stroke="#e0e0e0" |
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.
Avoid using inline CSS attributes as much as possible. Create a CSS class instead. There are a lot of places that need attention. For example:
BAD:
<circle stroke="#e0e0e0" opacity="0.7" cx=@centerX />
GOOD:
<circle class="rz-spider-chart-grid-circle" cx=@centerX />
var xStr = x.ToString("F2", System.Globalization.CultureInfo.InvariantCulture); | ||
var yStr = y.ToString("F2", System.Globalization.CultureInfo.InvariantCulture); | ||
|
||
@((MarkupString)$"<text x=\"{xStr}\" y=\"{yStr}\" text-anchor=\"{anchor}\" dominant-baseline=\"{baseline}\" class=\"rz-spider-chart-label\" style=\"fill: var(--rz-text-color); pointer-events: none;\">{category}</text>") |
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.
MarkupString isn't needed here. We have eliminated all its usage so we can't accept code that uses it.
|
||
@if (Series.Count > 2) | ||
{ | ||
<input type="text" placeholder="Filter..." |
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.
We don't need filter as you type for the legend items. Remove that and the related code. Leave only filter on click as in RadzenChart.
builder.OpenElement(0, "div"); | ||
builder.AddAttribute(1, "class", "rz-chart-tooltip-content"); | ||
builder.AddAttribute(2, "class", $"rz-series-{series.ColorIndex}-tooltip"); | ||
builder.AddAttribute(3, "style", "background: var(--rz-base-background-color); padding: 8px; border-radius: 4px; min-width: 150px; border-width: 2px; border-style: solid;"); |
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.
Do not use inline CSS styles here.
{ | ||
var value = series.Data.ContainsKey(category) ? series.Data[category] : 0; | ||
|
||
builder.OpenElement(index++, "div"); |
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.
Do not use index++
when creating elements. Use the usual index numbering in the for loop - it will still work.
@@ -0,0 +1,31 @@ | |||
namespace RadzenBlazorDemos.Pages |
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.
All demo code must be in a single .razor
file. Move the C# code in a @code
block inside SpiderChartBasic.razor
new PerformanceData | ||
{ | ||
Department = "ehab", | ||
DepartmentName = "Ehab Hussein", |
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.
Please do not use your name or the name of real people as sample data. Use names from the Northwind Employees table (Andrew Fuller, Nancy Davolio etc).
</g> | ||
</svg> | ||
|
||
@if (ShowLegend && Series?.Any() == true) |
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.
There is a very high chance people would request legend positioning similar to RadzenChart.
@akorchev Ill address all these and commit over the weekend. |
This PR introduces a new RadzenSpiderChart component to the Radzen Blazor library. Spider charts (also known as radar charts) are useful for displaying multivariate data in a radial format, making it easy to compare multiple metrics across different categories.