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

[Anchor] Support left / right direction (pt. 4 - add horizontal placement prop on Popover) #393

Merged
merged 7 commits into from
Oct 27, 2022

Conversation

chenesan
Copy link
Contributor

@chenesan chenesan commented Oct 26, 2022

Purpose

讓 popover 可以套用 left / right 的 placement,並加入 anchored right popover 的 story。
截圖 2022-10-26 上午12 01 32

Changes

  • a list of what have been done
  • maybe some code change

Risk

Usually none, if you have any please write it here.

TODOs

  • Describe what should be done outside of this PR
  • Maybe in other PRs or some manual actions.

@github-actions github-actions bot temporarily deployed to canary October 26, 2022 01:47 Inactive
@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #393 (1bd91ac) into develop (2e03919) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #393      +/-   ##
===========================================
+ Coverage    88.45%   88.46%   +0.01%     
===========================================
  Files          151      151              
  Lines         1533     1535       +2     
  Branches       278      280       +2     
===========================================
+ Hits          1356     1358       +2     
  Misses         177      177              
Impacted Files Coverage Δ
packages/core/src/mixins/anchored/index.js 78.37% <ø> (ø)
packages/core/src/Popover.js 82.35% <100.00%> (+2.35%) ⬆️
...kages/core/src/mixins/anchored/getPositionState.js 100.00% <100.00%> (ø)

@chenesan chenesan force-pushed the feature/add_right_placement_strategy_pt2 branch from 26653f4 to 25447fd Compare October 26, 2022 07:46
@chenesan chenesan self-assigned this Oct 26, 2022
@github-actions github-actions bot temporarily deployed to canary October 26, 2022 07:48 Inactive
@github-actions github-actions bot temporarily deployed to canary October 26, 2022 07:53 Inactive
@github-actions github-actions bot temporarily deployed to canary October 26, 2022 08:05 Inactive
@@ -48,7 +49,11 @@ function Popover({
* The `maxHeight` is for `BEM.container`, which doesn't include root class padding.
* So we need to minus POPOVER_PADDING here.
*/
const maxHeight = remainingSpace ? remainingSpace - POPOVER_PADDING : undefined;
const maxHeight = (
(remainingSpace && verticalPlacements.includes(placement))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

置左 / 右時就不需要指定 max-height 了

@chenesan chenesan marked this pull request as ready for review October 26, 2022 08:25
Copy link
Contributor

@zhusee2 zhusee2 left a comment

Choose a reason for hiding this comment

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

讚讚

Base automatically changed from feature/add_right_placement_strategy_pt1 to develop October 27, 2022 08:40
@github-actions github-actions bot temporarily deployed to canary October 27, 2022 08:43 Inactive
@chenesan chenesan merged commit c22e880 into develop Oct 27, 2022
@chenesan chenesan deleted the feature/add_right_placement_strategy_pt2 branch October 27, 2022 08:47
@chenesan chenesan mentioned this pull request Nov 4, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants