-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: remove tailwind #1453
Conversation
…necessary custom styled components
…ingular button, semantic improvements
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.
Very good! That's a lot of work 💯
The PR is so big, I really cant properly review it.
- This must be very thoroughly tested in all the layouts we have, especially SIMPOS. Our track record with big CCS changes is not that great 😛
- Not sure I see the benefit of using
<Stack>
for everything. When its just a simple div with flex. Seems like a unnecessary wrapper for many of the places it's used.
@soofstad Thank you reviewing! It was hard to split in to multiple PR's because it's so connected and it was hard to see if everything was correct until I actually stripped everything away. It's also impossible to read! When you edit wrappers like this it's almost impossible to see the actual changes, but I tried to write the PR description to contain everything that's been done. I tried not using unnecessary stacks, maybe I missed a few things that can be grouped with a simple div - will definitely take a look! Truuee, our track record isn't that great, but I have tested a lot! Will pay close attention and test more. I have a PR coming in SIMPOS and will update dm-create-app as well 🙂 |
What does this pull request change?
both packages and example application
dm-core
dm-core-plugins
Why is this pull request needed?