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

Fixed the issue with the variant of sort button. #2

Merged
merged 4 commits into from Oct 14, 2021
Merged

Fixed the issue with the variant of sort button. #2

merged 4 commits into from Oct 14, 2021

Conversation

ashfaqnisar
Copy link

@ashfaqnisar ashfaqnisar commented Oct 13, 2021

Hey @zxhmike, Awesome work buddy.
In mui5, by default, the button is taking the primary colour rather than no colour like mui4.
Due to that, the sort button in the table is using the primary variant rather than the default variant. So fixed the issue with the variant of the button.
Present:
image
Expected:
image

@ashfaqnisar
Copy link
Author

The sorted label of the table was not correctly aligned with the content of the table.
Extra padding(16px) was added in the mui5 version decreased it to 8px.
Present:
Screenshot 2021-10-13 122202
Expected:
image

@zxhmike
Copy link

zxhmike commented Oct 13, 2021

@ashfaqnisar Thank you so much for the contribution. I checked and it is looking great. Before we merge this PR, would you mind checking if there are other buttons that have the same situation? Thank you.

@ashfaqnisar
Copy link
Author

Hey @zxhmike, would be happy to do that!

@ashfaqnisar
Copy link
Author

Hello @zxhmike,
In the mui5, the default variant of the textfield, select & formcontrol is outlined. So, changed the variant to standard.

This fixed the issue of the misalignment of the filter component label with the select field.

@ashfaqnisar
Copy link
Author

Hey @zxhmike,
The search icon of the TableSearch was a bit misaligned with the text field & other icons. So, centred it.
This may be misaligned due to the reduction of the padding of IconButton from 48px to 40px in mui5.
I have added images for reference.
Previous Version:
image

Updated Version:
Screenshot 2021-10-14 104652

@ashfaqnisar
Copy link
Author

Hey @zxhmike,
Apart from these issues, I was not able to find any other issues for the time being. I think this PR is good to go. Will push a commit, if I find something!

@zxhmike zxhmike merged commit 9b70f9c into pisrcio:mui5 Oct 14, 2021
@zxhmike
Copy link

zxhmike commented Oct 14, 2021

Thank you @ashfaqnisar . The PR is merged.

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