-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 overflow, alignment and padding in text and button elements #7541
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7541 +/- ##
==========================================
+ Coverage 56.16% 56.20% +0.03%
==========================================
Files 672 672
Lines 27112 27112
Branches 2635 2635
==========================================
+ Hits 15227 15237 +10
+ Misses 11560 11550 -10
Partials 325 325
see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
Let's make sure we add some minimal coverage. I think we could update the display layout visual test to add a condition widget
@unlikelyzero How can I help move this through? Should I add tests? I'm not sure how to check against what we already have vs. the delta for new. |
All css changes have been approved by @davetsay and @charlesh88 |
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 @charlesh88 .
One question. Have you tested this against Jasper's complex views? Not that it should block this PR but it may be good to see how this might affect those.
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.
@charlesh88 I checked the latest percy diffs and didn't see anything that seemed related to these changes, so this could use a visual test. Let's sync on it and we can push this through
@ozyx , I met with @charlesh88 and we discussed a simple test with 4 objects within a display layout: |
Closes #7540
Describe your changes:
overflow: hidden
to Condition Widget label and alphanumeric wrappers.All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist