test(module: drawer): Test, refactor and document Drawer #2833

kooliokey · 2022-10-23T04:51:38Z

Bug fix:

Content string would never render (wrong condition checked)

Refactors:

Remove this.
Remove code that didn't do anything helpful from my testing - will comment on inline
Moved code to make it more obvious only one condition can exist at a time - will comment on inline

Documentation site update to match available parameters

🤔 This is a ...

🔗 Related issue link

#2644

💡 Background and solution

📝 Changelog

Language	Changelog
🇺🇸 English	Fix bug where Content wouldn't render in Drawer if it was a string and not RenderFragment
🇨🇳 Chinese	修复如果内容是字符串而不是 RenderFragment，则内容不会在 Drawer 中呈现的错误

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

Doc is updated/provided or not needed
Demo is updated/provided or not needed
Changelog is provided or not needed

… be null or empty the drawer closes with or without this code in place.

github-actions · 2022-10-23T04:51:50Z

kooliokey · 2022-10-23T04:53:19Z

components/drawer/Drawer.razor.cs


            base.OnInitialized();
        }

        protected override void OnParametersSet()
        {
-            this.SetClass();
-            if (string.IsNullOrEmpty(Placement) && Placement != _originalPlacement)


This block and line 448 block that was removed didn't do anything when Placement was null or empty string. I tested with a without it and it behaved the same both ways.

I checked when it was added to the repo and it was from the start and also moved in a refactor to fix a bug. I double checked that bug and it is still fixed with this removed. It was related to multiple drawers open.

kooliokey · 2022-10-23T04:54:08Z

components/drawer/Drawer.razor.cs

@@ -502,37 +482,16 @@ private async Task HandleClose(bool isChangeByParamater = false)
            }
        }

-        private void CalcAnimation()


I moved this to be inline with where it was called to make it obvious only the width or height transition could exist at once due to it being dependent on Placement's value. It is moved to line 490 below

kooliokey · 2022-10-23T04:54:53Z

tests/AntDesign.TestKit/AntDesignTestBase.cs

@@ -22,6 +22,7 @@ public AntDesignTestBase(bool useMoq = false)
            //Needed for Tests using Overlay
            Services.AddScoped<AntDesign.JsInterop.DomEventService>(sp => new TestDomEventService(Context.JSInterop.JSRuntime, MockedDomEventListener));
            JSInterop.SetupVoid(JSInteropConstants.OverlayComponentHelper.DeleteOverlayFromContainer, _ => true);
+            JSInterop.Mode = JSRuntimeMode.Strict;


Added this so even if a test changes it, all other tests still have it set to the default Strict. Not 100% sure the lifetime of this service but figured it was a good precaution.

codecov · 2022-10-23T04:59:17Z

Codecov Report

Base: 43.08% // Head: 43.76% // Increases project coverage by +0.67% 🎉

Coverage data is based on head (e3d3e40) compared to base (f9c5f5c).
Patch coverage: 86.20% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2833      +/-   ##
==========================================
+ Coverage   43.08%   43.76%   +0.67%     
==========================================
  Files         544      544              
  Lines       25768    25743      -25     
  Branches      260      260              
==========================================
+ Hits        11103    11267     +164     
+ Misses      14625    14436     -189     
  Partials       40       40

Impacted Files	Coverage Δ
components/drawer/Drawer.razor.cs	`90.19% <85.71%> (+90.19%)`	⬆️
components/drawer/Drawer.razor	`100.00% <100.00%> (+100.00%)`	⬆️
components/date-picker/RangePicker.razor.cs	`53.79% <0.00%> (+0.33%)`	⬆️
components/core/Component/Overlay/Overlay.razor.cs	`76.74% <0.00%> (+1.16%)`	⬆️
components/core/JsInterop/JSInteropConstants.cs	`32.80% <0.00%> (+3.19%)`	⬆️
components/core/ComponentStatus.cs	`50.00% <0.00%> (+50.00%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

kooliokey · 2022-10-23T05:01:02Z

Codecov Report

Base: 43.08% // Head: 42.38% // Decreases project coverage by -0.70% ⚠️

Coverage data is based on head (7d4009f) compared to base (f9c5f5c).
Patch coverage: 86.20% of modified lines in pull request are covered.

Additional details and impacted files
☔ View full report at Codecov. 📢 Do you have feedback about the report comment? Let us know in this issue.

How did coverage go down? 😢

ElderJames · 2022-10-23T05:15:36Z

Yes, that's a weird statistic. It feels different than the statistics of the master branch

ElderJames · 2022-10-23T07:44:15Z

components/drawer/Drawer.razor.cs

        private string _zIndexStyle = "";

-        private void TriggerPlacementChangeCycleOnce()


@zxyao145 Please check if this method is useless.

Yes, this method will only be triggered when the Placement changes (line 394).

Thanks @zxyao145 , please help to confirm whether there is any problem after deletion.

kooliokey added 3 commits October 23, 2022 00:16

Drawer Tests

8834283

Remove code that doesn't appear to be needed. If you set Placement to…

75cb2ff

… be null or empty the drawer closes with or without this code in place.

Documentation update

7d4009f

pr-triage bot added the PR: unreviewed label Oct 23, 2022

kooliokey commented Oct 23, 2022

View reviewed changes

kooliokey added 2 commits October 23, 2022 01:03

Doc update

2298465

Docs update...including chinese

e3d3e40

ElderJames requested a review from zxyao145 October 23, 2022 07:43

ElderJames reviewed Oct 23, 2022

View reviewed changes

zxyao145 approved these changes Oct 23, 2022

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Oct 23, 2022

ElderJames approved these changes Oct 23, 2022

View reviewed changes

ElderJames merged commit d58bcc2 into ant-design-blazor:master Oct 23, 2022

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Oct 23, 2022

ElderJames mentioned this pull request Oct 28, 2022

Test Coverage Improvement #2644

Open

62 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(module: drawer): Test, refactor and document Drawer #2833

test(module: drawer): Test, refactor and document Drawer #2833

kooliokey commented Oct 23, 2022 •

edited

github-actions bot commented Oct 23, 2022 •

edited

kooliokey Oct 23, 2022

kooliokey Oct 23, 2022

kooliokey Oct 23, 2022

codecov bot commented Oct 23, 2022 •

edited

kooliokey commented Oct 23, 2022

Codecov Report

ElderJames commented Oct 23, 2022

ElderJames Oct 23, 2022

zxyao145 Oct 23, 2022

ElderJames Oct 23, 2022

		private string _zIndexStyle = "";

		private void TriggerPlacementChangeCycleOnce()

test(module: drawer): Test, refactor and document Drawer #2833

test(module: drawer): Test, refactor and document Drawer #2833

Conversation

kooliokey commented Oct 23, 2022 • edited

🤔 This is a ...

🔗 Related issue link

💡 Background and solution

📝 Changelog

☑️ Self Check before Merge

github-actions bot commented Oct 23, 2022 • edited

kooliokey Oct 23, 2022

Choose a reason for hiding this comment

kooliokey Oct 23, 2022

Choose a reason for hiding this comment

kooliokey Oct 23, 2022

Choose a reason for hiding this comment

codecov bot commented Oct 23, 2022 • edited

Codecov Report

kooliokey commented Oct 23, 2022

Codecov Report

ElderJames commented Oct 23, 2022

ElderJames Oct 23, 2022

Choose a reason for hiding this comment

zxyao145 Oct 23, 2022

Choose a reason for hiding this comment

ElderJames Oct 23, 2022

Choose a reason for hiding this comment

kooliokey commented Oct 23, 2022 •

edited

github-actions bot commented Oct 23, 2022 •

edited

codecov bot commented Oct 23, 2022 •

edited