-
Notifications
You must be signed in to change notification settings - Fork 511
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
fix: fix the button margin space #1604
Conversation
also cc @guoqqqi to take a look |
wait for #1608 |
Codecov Report
@@ Coverage Diff @@
## master #1604 +/- ##
==========================================
+ Coverage 71.05% 71.24% +0.19%
==========================================
Files 47 47
Lines 3116 3116
==========================================
+ Hits 2214 2220 +6
+ Misses 658 653 -5
+ Partials 244 243 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
LGTM expect a small detail:
It would be better to remove root dom <></>
(https://github.com/apache/apisix-dashboard/pull/1604/files#diff-157cae0de0314d396c74fbb89c3f87b9b9c503826a29b3452e80d66a36a37120R63 and https://github.com/apache/apisix-dashboard/pull/1604/files#diff-157cae0de0314d396c74fbb89c3f87b9b9c503826a29b3452e80d66a36a37120R96), for <Space></Space>
has already wrapped all the buttons.
Hi @liuxiran Update the commit. |
Please answer these questions before submitting a pull request
Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
Fix There have 2 buttons fit very close together #1602
Bugfix
Use and
Space
to add each button's margin, and remove inline margin style.