-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
MudTreeView: Add AutoExpand, ExpandAll() and CollapseAll() #8762
MudTreeView: Add AutoExpand, ExpandAll() and CollapseAll() #8762
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #8762 +/- ##
==========================================
+ Coverage 89.82% 90.11% +0.28%
==========================================
Files 412 419 +7
Lines 11878 12175 +297
Branches 2364 2397 +33
==========================================
+ Hits 10670 10971 +301
+ Misses 681 660 -21
- Partials 527 544 +17 ☔ View full report in Codecov by Sentry. |
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.
You are going to hate me after this review
No ;) I just hate how braces for single line ifs and empty lines waste vertical space. I really don't understand why we need this style code |
@danielchalmers To make a clear point, let me answer a bit longer. I don't hate anybody for reviewing. I am grateful for everyone who reviews, but I don't have to agree with everything either. I respect all the work you have done for MudBlazor, I respect the skills and expertise you obviously have in many areas. I also admire your ability to hone and polish something until it is truly perfect. And if your critisism makes sense to me I'll be on your side. That said, I also respectfully refuse to do busywork that I think is pointless. Like adding or removing empty lines or adding braces where they aren't needed. I hope you'll forgive me for that ;) Sometimes I think @ScarletKuro adds more empty lines in his methods than actual lines of code, but I sure as hell won't start requesting him to remove them just because my coding style is different and I like methods without any empty lines. I can read his code just fine. Everything else would be a serious skill issue on my part. And it would be plain wrong on so many levels to impose my preferences on Kuro:
I want that we focus on productive changes and that we don't use our time for polishing the wrong stuff. It doesn't matter for MudBlazor how blank lines or braces are set in the code. I am all for polishing things that make a difference, such as public API consistency, the UI design, overall functionality, etc. I hope we can agree on that? Of course, please continue to review and I'll of course gladly accept any suggestion that has obvious merit. |
@henon Sure! In reality I'm just relaying what @ScarletKuro has taught me about his expectations for the code. I don't have any strong feelings either way. I thought you were on the same page which is why I was pointing out the discrepancies which I thought you missed accidentally! Will keep that in mind. Again, I'm not that concerned either way, just trying to help keep the quality up and style maintained. |
OK, great. I still have to write unit tests for this thing. |
Damn, a whole discussion. I just prefer to use in open source what most people are comfortable with, and my observations from discussions, forums, reddit, stackoverflow etc - it's always brackets, because it's clearer at a glance, it is less prone to error during revision etc. It's also funny that MS added this syntax, but doesn't allow it in most of their projects that I worked with, and I have this mindset that Microsoft is a golden standard in .NET world. But I'm obviously not going to fight over it with a team member 😂 |
Sorry for making a scene yesterday, it obviously riled me. OK, I don't have to love how much screen space a method with a couple of simple ifs and returns will waste on the screen but I can absolutely work with it. And I don't have to manually add all those braces. I can just let Resharper do it for me: |
2216c79
to
3a27018
Compare
Description
New features:
AutoExpand
will automatically and recursively expand down the tree until the selected item is visible. This of course happens only when itsSelected
state changes from false to true due to a change inSelectedValue
orSelectedValues
.ExpandAll()
recursively expands all itemsCollapseAll()
recursively collapses all itemsHow Has This Been Tested?
unit | visually
Type of Changes
Checklist
dev
).