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.

Sign up for GitHub

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

Jump to bottom

feat: add tabbar component horizontal-scroll-indicator #4979

Merged
merged 12 commits into from Dec 21, 2023
Merged

feat: add tabbar component horizontal-scroll-indicator #4979

merged 12 commits into from Dec 21, 2023

Conversation

AeroWang
Copy link
Contributor

@AeroWang AeroWang commented Dec 3, 2023 •
edited by ruibaby

What type of PR is this?

/area console
/kind improvement

What this PR does / why we need it:

在 Tabbar 组件内容可滚动时，添加内容超出时的水平方向滚动指示器；解决由 #4582 指出的体验问题

Which issue(s) this PR fixes:

Fixes #4582

Special notes for your reviewer:

注意观察各处使用 Tabbar 组件且内容可滚动时的情况（浏览器宽度变化也可生效）

Does this PR introduce a user-facing change?

添加 Tabbar 组件内容超出时的水平方向滚动指示器

@f2c-ci-robot f2c-ci-robot bot added area/console Issues or PRs related to the Halo Console release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/improvement Categorizes issue or PR as related to a improvement. labels Dec 3, 2023
@CLAassistant
Copy link

CLAassistant commented Dec 3, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@codecov Codecov
Copy link

codecov bot commented Dec 3, 2023 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b8d5d1f) 55.91% compared to head (484b47e) 55.91%.

Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #4979   +/-   ##
=========================================
  Coverage     55.91%   55.91%           
  Complexity     3032     3032           
=========================================
  Files           524      524           
  Lines         17826    17826           
  Branches       1329     1329           
=========================================
  Hits           9968     9968           
  Misses         7309     7309           
  Partials        549      549

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AeroWang AeroWang requested a review from ruibaby December 6, 2023 14:53
ruibaby
ruibaby reviewed Dec 8, 2023
View reviewed changes
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

当前的更改似乎有一些功能问题。

  1. 首次加载不会显示指示器，需要滚动之后才能显示。
  2. 点击指示器可能无法滚动。
  3. 滚动期间，指示器显示的时机似乎也有点问题。

视频：

Screen.Recording.2023-12-08.at.23.17.22.mov

@AeroWang
Copy link
Contributor Author

AeroWang commented Dec 8, 2023 •
edited

@ruibaby
很奇怪哦，是我调试的方法有问题吗？packages/components 目录内 pnpm dev，console 目录下 pnpm dev，每次我本地会出现各种问题，然后调通后，推送，下次再启动就又出现问题了
Suggestion

@AeroWang
fix: 箭头显示时机等问题
9a2ffd0 
集中改变 arrow 的显示隐藏
@AeroWang
Copy link
Contributor Author

AeroWang commented Dec 9, 2023

现在我本地是这样的。

视频

tabbar.mov

@ruibaby
Copy link
Member

ruibaby commented Dec 9, 2023

cc @LIlGG 有空帮忙 review 一下。

@ruibaby ruibaby requested a review from LIlGG December 9, 2023 11:39
@ruibaby
Copy link
Member

ruibaby commented Dec 9, 2023

@ruibaby 很奇怪哦，是我调试的方法有问题吗？packages/components 目录内 pnpm dev，console 目录下 pnpm dev，每次我本地会出现各种问题，然后调通后，推送，下次再启动就又出现问题了

现在 components 库已经支持了 storybook，你可以在 components 目录直接用 pnpm storybook 启动，调试起来会方便很多。

LIlGG
LIlGG requested changes Dec 10, 2023
View reviewed changes
Copy link
Member

@LIlGG LIlGG left a comment

Choose a reason for hiding this comment

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

感谢 @AeroWang 提出的 PR，我发现代码中有以下几处可能需要修正的位置。

console/packages/components/src/components/tabs/Tabbar.vue Outdated Show resolved Hide resolved
console/packages/components/src/components/tabs/Tabbar.vue Outdated Show resolved Hide resolved
console/packages/components/src/components/tabs/Tabbar.vue Outdated Show resolved Hide resolved
console/packages/components/src/components/tabs/Tabbar.vue Outdated Show resolved Hide resolved
console/packages/components/src/components/tabs/Tabbar.vue Outdated Show resolved Hide resolved
@AeroWang AeroWang requested a review from LIlGG December 10, 2023 05:53
@LIlGG
Copy link
Member

LIlGG commented Dec 10, 2023

对于弹窗类型的，似乎显示有些问题。

tabbar

@AeroWang
Copy link
Contributor Author

对于弹窗类型的，似乎显示有些问题。

监听宽度大约有 1px 的误差抖动，已修复

@AeroWang
Copy link
Contributor Author

这个 PR 是还有什么问题吗？

ruibaby
ruibaby reviewed Dec 21, 2023
View reviewed changes
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/approve

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 21, 2023
LIlGG
LIlGG approved these changes Dec 21, 2023
View reviewed changes
Copy link
Member

@LIlGG LIlGG left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Dec 21, 2023
@f2c-ci-robot f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LIlGG, ruibaby

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot merged commit 61c4a22 into halo-dev:main Dec 21, 2023
4 checks passed
@JohnNiang
Copy link
Member

/meow

@f2c-ci-robot f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 21, 2023

@JohnNiang: cat image

In response to this:

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@JohnNiang JohnNiang added this to the 2.12.x milestone Dec 21, 2023
@AirboZH
Copy link
Member

AirboZH commented Dec 21, 2023

/joke

@f2c-ci-robot f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 21, 2023

@AirboZH: What do you get when you cross a rabbit with a water hose? Hare spray.

In response to this:

/joke

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@AeroWang AeroWang deleted the feat/tabbar-scroll-indicator branch December 21, 2023 06:59
@JohnNiang JohnNiang modified the milestones: 2.12.x, 2.12.0 Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruibaby ruibaby ruibaby approved these changes

@LIlGG LIlGG LIlGG approved these changes

@guqing guqing Awaiting requested review from guqing

@JohnNiang JohnNiang Awaiting requested review from JohnNiang

Assignees

@LIlGG LIlGG

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/console Issues or PRs related to the Halo Console kind/improvement Categorizes issue or PR as related to a improvement. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Milestone
2.12.0
Development

Successfully merging this pull request may close these issues.

[Feature] 调整配置选单 UI
6 participants
@AeroWang @CLAassistant @ruibaby @LIlGG @JohnNiang @AirboZH