Skip to content

feat: New tabs #265

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
Merged
zombieJ merged 127 commits into from
May 28, 2020
Merged

feat: New tabs #265

zombieJ merged 127 commits into from
May 28, 2020

Conversation

@zombieJ
Copy link
Member

@zombieJ zombieJ commented May 21, 2020 •
edited
Loading

还需要改造支持移动设备滚动，交互可以先看起来。

ref ant-design/ant-design#24284

Preview：https://tabs-git-new-tabs.react-component.now.sh/iframe.html?id=rc-tabs--mix

  • Hover to open
  • ... follow the last tab

@zombieJ zombieJ requested a review from afc163 May 21, 2020 13:52
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 21, 2020

This pull request fixes 8 alerts when merging 317fb40 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging 3c006c7 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging d08b801 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging 9a12c56 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging 035d5e1 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging e3d3c15 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging b550452 into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request introduces 1 alert and fixes 8 when merging c0d4344 into b785993 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request introduces 1 alert and fixes 8 when merging 8cc8ac7 into b785993 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 28, 2020

This pull request fixes 8 alerts when merging 202651b into b785993 - view on LGTM.com

fixed alerts:

  • 3 for Superfluous trailing arguments
  • 2 for Potentially inconsistent state update
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Duplicate property

@zombieJ zombieJ merged commit 8165c76 into master May 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the new-tabs branch May 28, 2020 09:18
Tav0
Tav0 reviewed Jun 15, 2020
View reviewed changes
src/Tabs.tsx
setMergedActiveKey(tabs[newActiveIndex]?.key);
}
setActiveIndex(newActiveIndex);
}, [tabs.map(tab => tab.key).join('_'), mergedActiveKey, activeIndex]);
Copy link

@Tav0 Tav0 Jun 15, 2020

Choose a reason for hiding this comment

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

This is a potential bug, it is creating a new array every time thus triggering this useEffect more than needed

Copy link
Member Author

@zombieJ zombieJ Jun 16, 2020

Choose a reason for hiding this comment

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

array is join to string, not count array as condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@afc163 afc163 afc163 approved these changes

+1 more reviewer

@Tav0 Tav0 Tav0 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zombieJ @afc163 @Tav0