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

Replace "Cancel Adding Provider" SVG with an IconButton #554

Merged
merged 3 commits into from Apr 3, 2024

Conversation

Amnish04
Copy link
Collaborator

@Amnish04 Amnish04 commented Apr 3, 2024

As discussed in #530 (comment), we should not use Svg's directly as an alternative of a button since it is semantically incorrect.

https://www.reddit.com/r/webdev/comments/r72zq6/comment/hmwvobm/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button

I tried preserving the look of the button, and was successful for the most part.

Normal:
image

Active:
image

This fixes #551

@Amnish04 Amnish04 added this to the Release 1.9 milestone Apr 3, 2024
@Amnish04 Amnish04 self-assigned this Apr 3, 2024
Copy link

cloudflare-pages bot commented Apr 3, 2024

Deploying chatcraft-org with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0a573b9
Status: ✅  Deploy successful!
Preview URL: https://1522b6ab.console-overthinker-dev.pages.dev
Branch Preview URL: https://amnish04-issue-551.console-overthinker-dev.pages.dev

View logs

@Amnish04 Amnish04 modified the milestones: Release 1.9, Release 1.8 Apr 3, 2024
@Amnish04
Copy link
Collaborator Author

Amnish04 commented Apr 3, 2024

@kliu57 I have fixed the alignment issue you mentioned

@Amnish04 Amnish04 requested a review from kliu57 April 3, 2024 21:13
Copy link
Collaborator

@kliu57 kliu57 left a comment

Choose a reason for hiding this comment

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

it's causing the column headings as well as the data underneath the new row to shift to the right the moment we add the new row. I think it is due to the padding/margin of the iconbutton being greater than the padding/margin around the checkboxes. Can you fix it so there is no shift?

@Amnish04
Copy link
Collaborator Author

Amnish04 commented Apr 3, 2024

@kliu57 Just pushed the fix for that

@kliu57 kliu57 self-requested a review April 3, 2024 22:58
Copy link
Collaborator

@kliu57 kliu57 left a comment

Choose a reason for hiding this comment

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

looks good to me

@Amnish04
Copy link
Collaborator Author

Amnish04 commented Apr 3, 2024

Thanks for reviewing

@Amnish04 Amnish04 merged commit 8763c39 into main Apr 3, 2024
4 checks passed
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.

User settings providers table - new row close icon html fix
2 participants