-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Simplify style #784
Simplify style #784
Conversation
✅ Deploy Preview for bldrs-share ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Looks great!
As discussed:
- Add padding below dialog buttons
- Shared corner style for Notes and other components
Let me take one more look when you've done this to QA
Also as separate PR, let's move to mui icons
FYI, this is failing tests because you've changed the label for the sample IFC models. The E2E tests are looking for |
@oogali changed the labels for the sample project, but the tests are still failing. |
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.
Getting a lot of errors related to loaders.
@pablo-mayrgundter can you please take a look, when you have a chance, if it is not too distracting.
Getting a lot of errors related to loaders. @pablo-mayrgundter can you please take a look, when you have a chance, if it is not too distracting. Can you send more context? I just checked out this PR and it builds and tests fine. |
af1a913
to
a2c95df
Compare
@pablo-mayrgundter PTAL |
4cea762
to
9d0da06
Compare
@pablo-mayrgundter PTAL - all the tests are passing and the branch is up to date. |
Only quirk is not all boxes have same corners. In NavTree, the TreeItems are still rounded. I personally prefer keeping the rounding on text at least. Perhaps also on buttons. Also would be nice to have Open tooltip line up to either cover "Search" or line up nicely Go ahead and submit if you'd like and we can fix these up after |
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.
See my last comment
This PR is meant to simplify styles to follow MUI conventions.