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: tw.apply for component like styles #83

Merged
merged 26 commits into from
Jan 17, 2021
Merged

feat: tw.apply for component like styles #83

merged 26 commits into from
Jan 17, 2021

Conversation

sastan
Copy link
Collaborator

@sastan sastan commented Jan 14, 2021

See this #83 (comment) to try it out.

Resolve #73

@coveralls
Copy link

coveralls commented Jan 14, 2021

Pull Request Test Coverage Report for Build 491375292

  • 233 of 234 (99.57%) changed or added relevant lines in 7 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.2%) to 95.61%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/twind/presedence.ts 90 91 98.9%
Files with Coverage Reduction New Missed Lines %
src/css/index.ts 2 96.65%
src/twind/configure.ts 2 97.28%
Totals Coverage Status
Change from base Build 490136107: 0.2%
Covered Lines: 4272
Relevant Lines: 4463

💛 - Coveralls

@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2021

size-limit report 📦

Path Size
dist/twind.js 11.9 KB (+3.03% 🔺)

@github-actions
Copy link
Contributor

Try the Preview Package

Official releases are only available on registry.npmjs.org as twind.

This PR has been published to npm.pkg.github.com as @tw-in-js/twind@pr83.

Install/Update

Configure your NPM client (click to expand)

Adjust you .npmrc to use https://npm.pkg.github.com for @tw-in-js

@tw-in-js:registry=https://npm.pkg.github.com

Using the command line:

npm config set @tw-in-js:registry https://npm.pkg.github.com --global
# For npm
npm install --force twind@npm:@tw-in-js/twind@pr83

# For yarn - upgrade implies install
yarn upgrade twind@npm:@tw-in-js/twind@pr83

@sastan
Copy link
Collaborator Author

sastan commented Jan 14, 2021

Docs will follow. The api is as discussed in the issue.

@sastan sastan force-pushed the component-api branch 2 times, most recently from c462263 to 5915d0b Compare January 15, 2021 21:44
@sastan sastan force-pushed the main branch 2 times, most recently from d3d2fb6 to a63ed92 Compare January 15, 2021 21:55
Copy link
Contributor

@itsMapleLeaf itsMapleLeaf left a comment

Choose a reason for hiding this comment

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

Also, is there any concern with apply overwriting the existing Function.prototype.apply in JS? I suppose call is an alternative, so perhaps not


<details><summary>Using within <code>css</code> – pending</summary>

`tw.apply` can be used with `css` ( (_pending variable arguments, array support_):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it still pending? Just to make sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Jep. Working on a initial draft.

docs/components.md Outdated Show resolved Hide resolved
Copy link
Member

@rschristian rschristian left a comment

Choose a reason for hiding this comment

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

All of this is super nit-picky documentation grammar/capitalization changes. I'll try to run through the other docs at some point too to ensure consistency.

The recurring two are "tailwind" -> "Tailwind" and "css" -> "CSS". Tailwind refers to itself as "Tailwind" and CSS is an acronym, so it should be capitalized.

docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
docs/components.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@sastan
Copy link
Collaborator Author

sastan commented Jan 16, 2021

Also, is there any concern with apply overwriting the existing Function.prototype.apply in JS? I suppose call is an alternative, so perhaps not

That is definitely the case. It may look odd for some developers. What use case would there be to use Function.prototype.apply on tw?

I'm happy to use a different name but i thought we agreed on tw.apply to mirror tailwinds @apply. What other name would you suggest?

@sastan
Copy link
Collaborator Author

sastan commented Jan 16, 2021

All of this is super nit-picky documentation grammar/capitalization changes. I'll try to run through the other docs at some point too to ensure consistency.

That would be great. English is not my native language.

The recurring two are "tailwind" -> "Tailwind" and "css" -> "CSS". Tailwind refers to itself as "Tailwind" and CSS is an acronym, so it should be capitalized.

Jep.

@rschristian
Copy link
Member

The recurring two are "tailwind" -> "Tailwind" and "css" -> "CSS". Tailwind refers to itself as "Tailwind" and CSS is an acronym, so it should be capitalized.

Jep.

From a quick scan I do believe most of the other docs already style these two like this. This page is just the odd one out.

sastan and others added 5 commits January 16, 2021 21:07
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
sastan and others added 6 commits January 16, 2021 21:08
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
@sastan
Copy link
Collaborator Author

sastan commented Jan 16, 2021

@rschristian Thanks for all the suggestion. You made it really easy for me. I just needed to confirm these 🚀

@rschristian
Copy link
Member

@rschristian Thanks for all the suggestion. You made it really easy for me. I just needed to confirm these 🚀

NP, glad it could be of use. I do think there's a handful more, probably hidden behind a "Load more..." box.

@itsMapleLeaf
Copy link
Contributor

That is definitely the case. It may look odd for some developers. What use case would there be to use Function.prototype.apply on tw?

I'm happy to use a different name but i thought we agreed on tw.apply to mirror tailwinds @apply. What other name would you suggest?

I agree, and I still think apply is the best name, it just occurred to me and I figured I'd bring it up just in case. I can't think of any cases either 🤷

sastan and others added 9 commits January 17, 2021 09:35
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
Co-authored-by: Ryan Christian <33403762+rschristian@users.noreply.github.com>
@sastan sastan linked an issue Jan 17, 2021 that may be closed by this pull request
@sastan sastan merged commit d00a9d2 into main Jan 17, 2021
@sastan sastan deleted the component-api branch January 17, 2021 20:16
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.

Inline plugin global styles Component Proposal
4 participants