-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
feat: update alert/report icons and column order #12081
Conversation
b43205c
to
b7371db
Compare
Does anyone else feel the size of the status icons looks kind of odd? The warning icon is too large while the others are too small. |
It looks like it's only warning that's a bit larger. I'm guessing it's the underlying SVGs size and viewBox properties. This may be intentional, to draw attention to the failed ones? @mihir174 can you confirm? |
Codecov Report
@@ Coverage Diff @@
## master #12081 +/- ##
==========================================
- Coverage 67.58% 63.69% -3.90%
==========================================
Files 959 959
Lines 47221 47243 +22
Branches 4620 4638 +18
==========================================
- Hits 31914 30090 -1824
- Misses 15195 16966 +1771
- Partials 112 187 +75
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
b7371db
to
c135f8e
Compare
isReportEnabled={isReportEnabled} | ||
viewBox={ | ||
lastStateConfig.name === 'alert-solid-small' | ||
? '-6 -6 24 24' | ||
: '0 0 24 24' |
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.
might be a better way to assign viewBox
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.
Why do we need to do this, anyway? The viewbox on that icon is 0 0 24 24
, is there something strange going on with the spacing?
disableSortBy: true, | ||
}: any) => { | ||
return lastEvalDttm | ||
? moment.utc(lastEvalDttm).local().format(DATETIME_WITH_TIME_ZONE) |
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.
Unrelated but we should update other lists where we use moment
to properly handle timezones
isReportEnabled={isReportEnabled} | ||
viewBox={ | ||
lastStateConfig.name === 'alert-solid-small' | ||
? '-6 -6 24 24' | ||
: '0 0 24 24' |
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.
Why do we need to do this, anyway? The viewbox on that icon is 0 0 24 24
, is there something strange going on with the spacing?
@riahk yea there is a strange spacing with |
730818b
to
249c91d
Compare
249c91d
to
7489f07
Compare
* upstream/master: (55 commits) feat(explore): time picker enhancement (apache#11418) feat: update alert/report icons and column order (apache#12081) feat(explore): metrics and filters controls redesign (apache#12095) feat(alerts/reports): add refresh action (apache#12071) chore: add latest tag action (apache#11148) fix(reports): increase crontab size and alert fixes (apache#12056) Small typo fix in Athena connection docs (apache#12099) feat(queries): security perm simplification (apache#12072) feat(databases): security perm simplification (apache#12036) feat(dashboards): security permissions simplification (apache#12012) feat(logs): security permissions simplification (apache#12061) chore: Remove unused CodeModal (apache#11972) Fix typescript error (apache#12074) fix: handle context-dependent feature flags in CLI (apache#12088) fix: Fix "View in SQLLab" bug (apache#12086) feat(alert/report): add 'not null' condition option to modal (apache#12077) bumping superset ui to 15.18 and deckgl to 0.3.2 (apache#12078) fix: Python dependencies in apache#11499 (apache#12079) reset active tab on open (apache#12048) fix: improve import flow UI/UX (apache#12070) ...
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Alert State:
Error
Success
On Grace
Working
Not Triggered
Report State
Success
Error
TEST PLAN
ADDITIONAL INFORMATION