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

feat(specialChars): Create and expose specialChars map #510

Merged
merged 5 commits into from Dec 14, 2020
Merged

feat(specialChars): Create and expose specialChars map #510

merged 5 commits into from Dec 14, 2020

Conversation

vasilii-kovalev
Copy link

@vasilii-kovalev vasilii-kovalev commented Dec 2, 2020

What: Create and expose specialChars map

Why:

  • It can help to avoid typos providing the constant values;
  • Instead of creating the set manually in each project again and again, it could be more convenient to use the official, always up-to-date one.

How:

  • Create a constant specialCharMap
  • Link it with specialCharCallbackMap
  • Export the constant from src/type.js and create a named export of the constant called specialChars in src/index.js
  • Add an appropriate enum in typings/index.d.ts called specialChars
  • Add specialChars section to README.md

Checklist:

  • Documentation
  • Tests
  • Typings
  • Ready to be merged

P.S.: I've defined enum because const is not allowed in index.d.ts. If src/type.js could be written in TypeScript, it could be possible to use a const assertion syntax instead, so the specialCharMap constant could be a single source of truth.

@codecov
Copy link

codecov bot commented Dec 2, 2020

Codecov Report

Merging #510 (de970c7) into master (9f401e8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #510   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          674       675    +1     
  Branches       211       211           
=========================================
+ Hits           674       675    +1     
Impacted Files Coverage Δ
src/index.js 100.00% <ø> (ø)
src/type.js 100.00% <100.00%> (ø)

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 9f401e8...de970c7. Read the comment docs.

@vasilii-kovalev
Copy link
Author

vasilii-kovalev commented Dec 2, 2020

Also type and specialChars sections should be in sync, but I can't figure out, how to conveniently combine them. The characters in the sections are not 100% the same actually (all the special characters - modifier ones). Maybe we can add a 5th column to the type section's table or something...

Copy link
Member

@marcosvega91 marcosvega91 left a comment

Choose a reason for hiding this comment

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

For me seems cool, thanks @vasilii-kovalev 😄 👍

Vasilii Kovalev added 2 commits December 3, 2020 10:08
Copy link
Member

@marcosvega91 marcosvega91 left a comment

Choose a reason for hiding this comment

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

For me is still ok, let's wait the opinion of someone else, thank you for your works 😄

@vasilii-kovalev
Copy link
Author

@kentcdodds, hello!
Could you take a look at this PR, please?

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

I guess I'm ok with this. I personally wouldn't use it, but I can understand why others might 👍

@kentcdodds
Copy link
Member

I'm good with this, but I'm going to let someone else merge it :)

@kentcdodds kentcdodds merged commit e8b7778 into testing-library:master Dec 14, 2020
@kentcdodds
Copy link
Member

@all-contributors please add @vasilii-kovalev for code and docs

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @vasilii-kovalev! 🎉

@kentcdodds
Copy link
Member

Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)

@github-actions
Copy link

🎉 This PR is included in version 12.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants