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: break path w/ <wbr>s to avoid copying ZWSPs #7516

Merged
merged 3 commits into from Sep 30, 2021

Conversation

MingweiSamuel
Copy link
Contributor

Description

  • Use instead of ZERO-WIDTH SPACE (U+200B) to break segments in
    operation-summary-path.jsx
  • Remove no-longer-needed onCopyCapture listener which previously
    stripped ZWSPs

Motivation and Context

Fixes #7513

How Has This Been Tested?

Basic local test, HTML looks right and behaves the same as before, just without copying of ZWSPs.

Screenshots (if appropriate):

N/A

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

MingweiSamuel added a commit to MingweiSamuel/swagger-ui that referenced this pull request Sep 17, 2021
- Use <wbr> instead of ZERO-WIDTH SPACE (U+200B) to break segments in
  operation-summary-path.jsx
- Remove no-longer-needed onCopyCapture listener which previously
  stripped ZWSPs

Closes swagger-api#7513
@MingweiSamuel MingweiSamuel changed the title fix: break path w/ <wbr>s to avoid copying ZWSPs (#7516) fix: break path w/ <wbr>s to avoid copying ZWSPs Sep 17, 2021
@char0n char0n self-assigned this Sep 18, 2021
@char0n char0n self-requested a review September 18, 2021 06:26
MingweiSamuel added a commit to MingweiSamuel/swagger-ui that referenced this pull request Sep 19, 2021
- Use <wbr> instead of ZERO-WIDTH SPACE (U+200B) to break segments in
  operation-summary-path.jsx
- Remove no-longer-needed onCopyCapture listener which previously
  stripped ZWSPs

Closes swagger-api#7513
Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

You did a great job. Can you please look at my code review comments before we move forward?

src/core/components/operation-summary-path.jsx Outdated Show resolved Hide resolved
src/core/components/operation-summary-path.jsx Outdated Show resolved Hide resolved
src/core/components/operation-summary-path.jsx Outdated Show resolved Hide resolved
src/core/components/operation-summary-path.jsx Outdated Show resolved Hide resolved
MingweiSamuel added a commit to MingweiSamuel/swagger-ui that referenced this pull request Sep 22, 2021
- Use <wbr> instead of ZERO-WIDTH SPACE (U+200B) to break segments in
  operation-summary-path.jsx
- Remove no-longer-needed onCopyCapture listener which previously
  stripped ZWSPs
- Update's deep-link.jsx's `text` prop type to accept `PropType.node`
  to allow the above.

Closes swagger-api#7513
@MingweiSamuel
Copy link
Contributor Author

Seems there is a warning I need to fix

Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `_class4`. It was passed a child from OperationSummaryPath. See https://fb.me/react-warning-keys for more information.

Probably a pretty basic react thing

- Use <wbr> instead of ZERO-WIDTH SPACE (U+200B) to break segments in
  operation-summary-path.jsx
- Remove no-longer-needed onCopyCapture listener which previously
  stripped ZWSPs
- Update's deep-link.jsx's `text` prop type to accept `PropType.node`
  to allow the above.

Closes swagger-api#7513
@MingweiSamuel
Copy link
Contributor Author

MingweiSamuel commented Sep 22, 2021

Replacing

pathParts.splice(i, 0, <wbr />)

with

pathParts.splice(i, 0, <wbr key={i} />)

Fixed the above. I guess this was in the original version of this PR too, just didn't notice.

Ready for another review.

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

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

We're looking good here. Thank you for your contribution!

@char0n char0n merged commit f88334a into swagger-api:master Sep 30, 2021
@MingweiSamuel
Copy link
Contributor Author

Thanks! 🎉

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

Successfully merging this pull request may close these issues.

ZERO WIDTH SPACE (U+200B) in path copied if selection is not exact
2 participants