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

fix: (Typography) renderCopy function has bug when child element is r… #688

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lio-mengxiang
Copy link

@lio-mengxiang lio-mengxiang commented Mar 7, 2022

…eact node

中文模板 / Chinese Template

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Test Case
  • TypeScript definition update
  • Document improve
  • CI/CD improve
  • Branch sync
  • Other, please describe:

PR description

Fixes #

Changelog

🇨🇳 Chinese

  • Fix: 修复 ...
  • 问题:
    在Typography 组件中,如果要的点击复制文字按钮,复制的文字,如果是ReactNode类型会有bug,案例如下:
// 点击copy的icon,实际上复制的文字是:“[object Object]”
<Paragraph copyable><span>复制的文字</span></Paragraph> 

// 或者如下,点击copy的icon,实际上复制的文字是:“复制文字[object Object]复制文字3”
<Paragraph copyable>复制文字<span>复制文字2<span>复制文字3</Paragraph>

解决方法:
我写了一个递归方法,逻辑是如果子元素,如上面的

<span>复制的文字</span>

这个children元素如果不是string或者number,就继续往下找,方法如下(我也是arco design Typography组件的贡献者,这个方法没啥问题,arco也在用):

const isSingleNode = (child: React.ReactNode) => {
   // isString和isNumber是loadsh里的方法
    return isString(child) || isNumber(child);
};

  
export function mergedToString(children: any): string {
    const mergedResult = [''];
    React.Children.forEach(children, (child) => {
        const prevIndex = mergedResult.length - 1;
        const prevChild = mergedResult[prevIndex];
  
        if (isSingleNode(child) && isSingleNode(prevChild)) {
            mergedResult[prevIndex] = `${prevChild}${child}`;
        } else if (child?.props?.children) {
            mergedResult.push(mergedToString(child.props.children));
        }
    });
  
    return mergedResult.join('');
}

🇺🇸 English

  • Fix: fix ...

  • question:
    In the Typography component, if you want to click the copy text icon, the copied text will have bugs if it is a ReactNode type. The case is as follows:

// Click on the copy icon, the actual copied text is :“[object Object]”
<Paragraph copyable><span>复制的文字</span></Paragraph> 
// Or as follows, click the icon of copy, the actual copied text is:“复制文字[object Object]复制文字3”
<Paragraph copyable>复制文字<span>复制文字2<span>复制文字3</Paragraph>

Solution:
I wrote a recursive method, the logic is that if the child element, such as 
```javascri[t
<span> copied text</span>

, if the children element is not a string or a number, continue to look down, the method is as follows (I am also arco designContributor of the Typography component, there is nothing wrong with this method, this method arco is also used):

const isSingleNode = (child: React.ReactNode) => {
   // isString和isNumber是loadsh里的方法
    return isString(child) || isNumber(child);
};

  
export function mergedToString(children: any): string {
    const mergedResult = [''];
    React.Children.forEach(children, (child) => {
        const prevIndex = mergedResult.length - 1;
        const prevChild = mergedResult[prevIndex];
  
        if (isSingleNode(child) && isSingleNode(prevChild)) {
            mergedResult[prevIndex] = `${prevChild}${child}`;
        } else if (child?.props?.children) {
            mergedResult.push(mergedToString(child.props.children));
        }
    });
  
    return mergedResult.join('');
}
### Checklist
- [x] Test or no need
- [x] Document or no need
- [ ] Changelog or no need

### Other
- [] Skip Changelog

### Additional information
<!-- You can provide screenshot/video or some additional information -->

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 7, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2efb581:

Sandbox Source
DouyinFE/semi-design Configuration
Semi Design: Simple Story Configuration

@codecov-commenter
Copy link

Codecov Report

Merging #688 (2efb581) into main (c003abc) will increase coverage by 0.02%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #688      +/-   ##
==========================================
+ Coverage   81.69%   81.72%   +0.02%     
==========================================
  Files         365      365              
  Lines       20492    20493       +1     
  Branches     5226     5226              
==========================================
+ Hits        16741    16747       +6     
+ Misses       3731     3726       -5     
  Partials       20       20              
Impacted Files Coverage Δ
packages/semi-ui/typography/util.tsx 13.92% <83.33%> (+12.43%) ⬆️
packages/semi-ui/typography/base.tsx 84.31% <100.00%> (+2.45%) ⬆️

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 c003abc...2efb581. Read the comment docs.

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.

2 participants