-
Notifications
You must be signed in to change notification settings - Fork 0
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: update tailwindcss #340
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I checked the PR you linked but there is no description of the problem, can you elaborate on the issue and the steps you took to try to fix it? (Like I got that there must be some sort of problem with postcssv8 but I don't quite understand what the problem is so far...)
-
Do we need to prepare the projects to support this so we are not blocked in making updates on the components or can we install the components library with tailwind v2
-
I assume you followed https://tailwindcss.com/docs/upgrading-to-v2 maybe you can add it to the description so reviews don't need to search for the breaking changes in the new version...
@@ -27,7 +27,7 @@ | |||
"peerDependencies": { | |||
"react": ">=16.12.0", | |||
"react-dom": ">=16.12.0", | |||
"tailwindcss": "^1.4.0" | |||
"tailwindcss": "npm:@tailwindcss/postcss7-compat@^2.1.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this forcing the resolution to another package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is telling tailwindcss to use PostCSS 7, they included PostCSS 7 compatibility build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's holding us back from using postcss 8?
I'm ok with doing an intermediary step, but I would not want to close the jira ticket with this workaround in place
@@ -102,7 +102,7 @@ class Menu<T> extends Component<Props<T>> { | |||
{...getMenuProps( | |||
{ | |||
className: classNames( | |||
"border border-grey-3 absolute z-dropdownMenu bg-white cursor-normal inset-x-0 list-reset scrolling-touch overflow-y-scroll custom-scrollbar max-h-dropdownSM md:max-h-dropdown py-10 shadow-soft rounded-4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this safe to drop regarding the browser support of the project, or does it only apply to iOS native apps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its not supported anymore, tailwindlabs/tailwindcss#2573
We will test it and see if it is still ok
I also dont know what is the problem, npm ls fails, but I cannot reproduce it on my computer https://app.circleci.com/pipelines/github/carforyou/carforyou-components-pkg/2558/workflows/a6ef542b-92cf-45fb-ac69-66b05f5216dc/jobs/8054
There are some breaking changes that we need to handle, will take care of it
You can also check the release notes here -> https://github.com/tailwindlabs/tailwindcss/releases/tag/v2.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm cool with unlocking the tailwind update with the compat package. it's a good isolated step on the upgrade path. I would not want to stop there though and push for postcss8 in the next PR
@@ -27,7 +27,7 @@ | |||
"peerDependencies": { | |||
"react": ">=16.12.0", | |||
"react-dom": ">=16.12.0", | |||
"tailwindcss": "^1.4.0" | |||
"tailwindcss": "npm:@tailwindcss/postcss7-compat@^2.1.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's holding us back from using postcss 8?
I'm ok with doing an intermediary step, but I would not want to close the jira ticket with this workaround in place
BREAKING CHANGE: update tailwindcss to 2.1.2
As you can see here #341, I think I have some issues updating postcss 8 and that's why I installed the version of tailwindcss that supports postcss 7
I will create another task to update postcss or feel free to help me with my current PR
Note: Before merging this PR will ask QA to test it on Listings and DH