You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/20 19:22:41 UTC
[GitHub] [incubator-superset] mistercrunch opened a new pull request #10971: style: improve default
mistercrunch opened a new pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971
## after
<img width="610" alt="Screen Shot 2020-09-20 at 12 21 09 PM" src="https://user-images.githubusercontent.com/487433/93720131-e6150180-fb3b-11ea-8479-fecd59164692.png">
## before
<img width="615" alt="Screen Shot 2020-09-20 at 12 20 03 PM" src="https://user-images.githubusercontent.com/487433/93720133-e8775b80-fb3b-11ea-98c1-8a9dab9684ea.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697186207
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=h1) Report
> Merging [#10971](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `5.10%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10971 +/- ##
==========================================
- Coverage 65.79% 60.68% -5.11%
==========================================
Files 816 383 -433
Lines 38422 24186 -14236
Branches 3621 0 -3621
==========================================
- Hits 25280 14678 -10602
+ Misses 13034 9508 -3526
+ Partials 108 0 -108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.68% <ø> (-0.72%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.53% <0.00%> (-11.39%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.74% <0.00%> (-1.66%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.56% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.92% <0.00%> (-0.25%)` | :arrow_down: |
| ... and [431 more](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=footer). Last update [d056e3d...d8d7b08](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697169042
<img width="562" alt="Screen Shot 2020-09-22 at 11 26 43 PM" src="https://user-images.githubusercontent.com/487433/93976594-0efce880-fd2e-11ea-89f1-8f48f0748af1.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697186207
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=h1) Report
> Merging [#10971](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/193796ca1615098ce97ee03a668a754206c17df5?el=desc) will **decrease** coverage by `0.08%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10971 +/- ##
==========================================
- Coverage 65.84% 65.75% -0.09%
==========================================
Files 815 816 +1
Lines 38358 38382 +24
Branches 3607 3606 -1
==========================================
- Hits 25258 25240 -18
- Misses 12992 13034 +42
Partials 108 108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `57.01% <100.00%> (-0.09%)` | :arrow_down: |
| #javascript | `61.72% <100.00%> (-0.01%)` | :arrow_down: |
| #python | `61.36% <ø> (-0.10%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...src/explore/components/controls/VizTypeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaXpUeXBlQ29udHJvbC5qc3g=) | `92.30% <ø> (ø)` | |
| [superset-frontend/src/components/Icon/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbi9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Label/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGFiZWwvaW5kZXgudHN4) | `100.00% <100.00%> (ø)` | |
| [.../explore/components/controls/DatasourceControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRhc291cmNlQ29udHJvbC5qc3g=) | `83.78% <100.00%> (ø)` | |
| [...rontend/src/visualizations/FilterBox/FilterBox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9GaWx0ZXJCb3guanN4) | `56.17% <0.00%> (-9.88%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `7.14% <0.00%> (-6.20%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| [.../src/dashboard/components/gridComponents/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `83.67% <0.00%> (-2.05%)` | :arrow_down: |
| [...rset-frontend/src/SqlLab/components/QueryTable.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5VGFibGUuanN4) | `60.86% <0.00%> (-1.93%)` | :arrow_down: |
| ... and [41 more](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=footer). Last update [193796c...34cfb3a](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696274724
@JunlinC I tagged `design-review` for input!
About the motives, I think the recently tweaked default <Label> is way too dark and "pops" way too much in this current state. The previous "white on a lighter gray" had usability issues, so I figure I'd go "black on light-gray with a light border" and it looks much better to me than before.
More generally, I'm not sure if the label-with-popover pattern for controls is the right approach. I'd be curious to see how it would look if we'd redesign it from scratch.
Open to suggestion & tweaks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697186207
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-695911225
Overall great improvements! But I'm a little worried that the interaction with DatasourceControl may have become a little more confusing. The gray pill looks the same like the clickable labels in other controls (Viz Type and Time Range) but is not actually clickable. Using the cog icon for a dropdown menu also seems a little anti-pattern. What do you think of keeping it as a button/label with a dropdown menu and limiting this PR to just changing the style?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
JunlinC commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696204836
may I humbly ask UI changes can somehow go thru Product and Design? 🙏 I was planning to work on those labels with Design next sprint. we received some feedback from multiple users ... @mistercrunch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696199353
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697186207
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=h1) Report
> Merging [#10971](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `5.05%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10971 +/- ##
==========================================
- Coverage 65.79% 60.74% -5.06%
==========================================
Files 816 383 -433
Lines 38422 24186 -14236
Branches 3621 0 -3621
==========================================
- Hits 25280 14691 -10589
+ Misses 13034 9495 -3539
+ Partials 108 0 -108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.74% <ø> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.53% <0.00%> (-11.39%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.56% <0.00%> (-0.83%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.70% <0.00%> (-0.14%)` | :arrow_down: |
| [.../src/SqlLab/components/EstimateQueryCostButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0VzdGltYXRlUXVlcnlDb3N0QnV0dG9uLmpzeA==) | | |
| [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | | |
| ... and [429 more](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=footer). Last update [d056e3d...d8d7b08](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
rusackas commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698112778
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
JunlinC commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696204836
may I humbly ask UI changes can somehow go thru Product and Design? 🙏 I was planning to work on those labels with Design next sprint. we received some feedback from multiple users ... @mistercrunch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698704425
Oh, reopening for things not covered in #11034
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch closed pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696274724
@JunlinC I tagged `design-review` for input! Happy to consider other approaches that address the underlying motives.
About the motives, I think the recently tweaked default <Label> is way too dark and "pops" way too much in this current state. The previous "white on a lighter gray" had usability issues, so I figure I'd go "black on light-gray with a light border" and it looks much better to me than before.
More generally, I'm not sure if the label-with-popover pattern for controls is the right approach. I'd be curious to see how it would look if we'd redesign it from scratch.
Open to suggestion & tweaks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696199353
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698716345
@rusackas - rebased!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
rusackas commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698159346
I also took some of your ideas and melded them into https://github.com/apache/incubator-superset/pull/11034 which uses your inverted/lightened default color, but treats the stroke/border idea differently, applying it to labels of _any_ color, but only when they have an `onClick` handler. Then, at least, the border systemically means "clickable".
That said, this is probably a stopgap solution in the longer term, and many of these clickable labels should be replaced by Buttons, Dropdowns, etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
rusackas commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698112778
There's [another PR ](https://github.com/apache/incubator-superset/pull/11033/)open where all the icons are moving over from the design system to the codebase. I'm trying to keep the names matching between Figma and the code, so there's a bit of overlap/misalignment with what's in this PR.
In this PR, you're adding a few icons, which are _also_ in the other one, but with different names. Would it be OK to ask for rebasing/renaming when that other PR slides in, lest they both merge and we have duplicates?
Mapping (names):
cog -> gear
minus-circle -> minus
plus-circle -> plus
FWIW, the emerging standard for naming seems to be underscores in filenames (partly thanks to Figma) and dashes in the name values, as you have it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696199353
I totally agree about the dataset label, maybe the label should have a caret and be a dropdown button, but it seemed harder than it should be to do that. About the 3 vertical dots?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-696199353
I totally agree about the dataset label, maybe the label should have a caret and be a dropdown button, but it seemed harder than it should be to do that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697169042
![Uploading Screen Shot 2020-09-22 at 11.26.43 PM.png…]()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-695911225
Overall great improvements! But I'm a little worried that the interaction with DatasourceControl may have become a little more confusing. The gray pill looks the same like the clickable labels in other controls (Viz Type and Time Range) but is not actually clickable. Using the cog icon for a dropdown menu also seems a little anti-pattern. What do you think of keeping it as a button/label with a dropdown menu and limiting this PR to just changing the style?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch closed pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
ktmud commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-695911225
Overall great improvements! But I'm a little worried that the interaction with DatasourceControl may have become a little more confusing. The gray pill looks the same like the clickable labels in other controls (Viz Type and Time Range) but is not actually clickable. Using the cog icon for a dropdown menu also seems a little anti-pattern. What do you think of keeping it as a dropdown menu and limiting this PR to just changing the style?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697153532
<img width="571" alt="Screen Shot 2020-09-22 at 11 05 49 PM" src="https://user-images.githubusercontent.com/487433/93972939-2fc23f80-fd28-11ea-9507-8b40fef3fa81.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch removed a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
mistercrunch removed a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697153532
<img width="571" alt="Screen Shot 2020-09-22 at 11 05 49 PM" src="https://user-images.githubusercontent.com/487433/93972939-2fc23f80-fd28-11ea-9507-8b40fef3fa81.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698456516
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10971: style: improve default
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-697186207
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=h1) Report
> Merging [#10971](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `4.67%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10971 +/- ##
==========================================
- Coverage 65.79% 61.12% -4.68%
==========================================
Files 816 816
Lines 38422 38419 -3
Branches 3621 3621
==========================================
- Hits 25280 23483 -1797
- Misses 13034 14750 +1716
- Partials 108 186 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.77% <100.00%> (ø)` | |
| #python | `60.74% <ø> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...src/explore/components/controls/VizTypeControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaXpUeXBlQ29udHJvbC5qc3g=) | `76.92% <ø> (-15.39%)` | :arrow_down: |
| [.../explore/components/controls/DatasourceControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRhc291cmNlQ29udHJvbC5qc3g=) | `64.86% <100.00%> (-18.92%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [175 more](https://codecov.io/gh/apache/incubator-superset/pull/10971/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=footer). Last update [d056e3d...d8d7b08](https://codecov.io/gh/apache/incubator-superset/pull/10971?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch merged pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
ktmud commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-695911225
Overall great improvements! But I'm a little worried that the interaction with DatasourceControl may have become a little more confusing. The gray pill looks the same like the clickable labels in other controls (Viz Type and Time Range) but is not actually clickable. Using the cog icon for a dropdown menu also seems a little anti-pattern. What do you think of keeping it as a dropdown menu and limiting this PR to just changing the style?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
mistercrunch commented on pull request #10971:
URL: https://github.com/apache/incubator-superset/pull/10971#issuecomment-698456516
closing in favor of https://github.com/apache/incubator-superset/pull/11034
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org