Skip to content
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

feat(model: overlay): add boundary detection adjustment #1109

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

mutouzdl
Copy link
Member

@mutouzdl mutouzdl commented Feb 7, 2021

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • Bundle size optimization
  • Performance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

#1081

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English add overlay boundary detection and orientation adjustment
🇨🇳 Chinese 增加overlay的边界检测和方向调整

☑️ 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

@codecov-io
Copy link

Codecov Report

Merging #1109 (6b080cf) into master (49a2d13) will decrease coverage by 0.01%.
The diff coverage is 2.56%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1109      +/-   ##
=========================================
- Coverage    6.60%   6.59%   -0.02%     
=========================================
  Files         436     436              
  Lines       24165   24233      +68     
=========================================
+ Hits         1596    1598       +2     
- Misses      22569   22635      +66     
Impacted Files Coverage Δ
components/core/Component/overlay/Overlay.razor.cs 10.85% <0.00%> (-1.37%) ⬇️
...nents/core/Component/overlay/type/PlacementType.cs 50.00% <0.00%> (-40.48%) ⬇️
components/menu/SubMenu.razor.cs 0.00% <ø> (ø)
...nts/core/Component/overlay/OverlayTrigger.razor.cs 10.31% <12.50%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49a2d13...6b080cf. Read the comment docs.

@mutouzdl mutouzdl changed the title feat(model: overlay): add boundary detection and orientation adjustment feat(model: overlay): add boundary detection adjustment Feb 8, 2021
Copy link
Member

@ElderJames ElderJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anddrzejb
Copy link
Member

@mutouzdl Sorry to bother you about this, but I am trying to work out the overlay boundary calculation. I cannot figure out what is the meaning of TriggerBoundaryAdjustMode.InScroll. Do you remember what was that? I think I possibly am able to simplify the calculations, but I cannot figure out what that enum value really stands for (or how different calculations should be). I cannot find it used anywhere...

@mutouzdl
Copy link
Member Author

mutouzdl commented Aug 7, 2021

@anddrzejb It's a little hard to understand, and I'll probably give you two examples to illustrate it.

Example1: In InScroll mode, in this case, the menu will appear on top.
This is because the browser is not high enough to fully display the menu below (even if the scroll bar is scrolled)
image

Example2: In InScroll mode, the following situation will display the menu below.
Although the browser height still doesn't seem to display the full menu, if you scroll the scroll bar, it will, so in this case, the menu will still expand below
image

If anything is unclear, I will try to explain further.

@anddrzejb
Copy link
Member

@mutouzdl Thanks for the explanation! I think I understand now. So, we have these scenarios for calculations:

  • TriggerBoundaryAdjustMode.InView - the default, the viewport boundaries are the boundaries we use for calculation. We try to fit the overlay so it is always fully visible in the viewport
  • TriggerBoundaryAdjustMode.InScroll - the document boundaries are the boundaries used for calculation. So even if the overlay is outside of the viewport, the overlay may still be shown as long as it does not "overflow" the document boundaries.

@mutouzdl
Copy link
Member Author

mutouzdl commented Aug 7, 2021

@anddrzejb Yes, your interpretation is correct, and your summary is very accurate!

@anddrzejb
Copy link
Member

I will add this or similar explanation to the docs once I finish.

@anddrzejb
Copy link
Member

@mutouzdl I have another question. I am trying to wrap my head around parameter ArrowPointAtCenter. What I can see here, this seems to me that it is doing exactly the same job as when setting one of the placements to the center placements (Left, Right, Bottom/BottomCenter or Top/TopCenter). Do I understand this correctly, or there is some more to this parameter that I am missing?

@mutouzdl
Copy link
Member Author

@mutouzdl I have another question. I am trying to wrap my head around parameter ArrowPointAtCenter. What I can see here, this seems to me that it is doing exactly the same job as when setting one of the placements to the center placements (Left, Right, Bottom/BottomCenter or Top/TopCenter). Do I understand this correctly, or there is some more to this parameter that I am missing?

@anddrzejb Yes, your understanding is correct.

ArrowPointAtCenter is used for ToolTip and other components.
TopLeft, for example, looks like this on ToolTip by default:
image

If ArrowPointAtCenter=true, the arrow will point to the center of the target element:
image

@anddrzejb
Copy link
Member

@mutouzdl I thought so. I was a bit confused by the calculation based on the arrow shifts. But the redesign will just switch internally to appropriate cantered placement I guess.

ElderJames pushed a commit that referenced this pull request Apr 23, 2022
* feat(model: overlay): add boundary detection and orientation adjustment

* feat: suport inview and inscroll mode
ElderJames pushed a commit that referenced this pull request Apr 30, 2022
* feat(model: overlay): add boundary detection and orientation adjustment

* feat: suport inview and inscroll mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants