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/07/17 05:50:53 UTC
[GitHub] [incubator-superset] mistercrunch opened a new pull request #10355: feat: SIP-34 explore save modal
mistercrunch opened a new pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355
Getting the explore save modal closer to SIP-34
## Before
<img width="682" alt="Screen Shot 2020-07-16 at 10 50 24 PM" src="https://user-images.githubusercontent.com/487433/87753127-c1f11900-c7b6-11ea-8845-a4b5472fc794.png">
## After
<img width="669" alt="Screen Shot 2020-07-16 at 10 47 09 PM" src="https://user-images.githubusercontent.com/487433/87753024-85bdb880-c7b6-11ea-9941-a105911ef83d.png">
## SIP-34
<img width="821" alt="Screen Shot 2020-07-16 at 10 49 23 PM" src="https://user-images.githubusercontent.com/487433/87753066-9e2dd300-c7b6-11ea-8ceb-913c87a686ff.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 edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661985391
Proposing two options:
- Bolded
<img width="674" alt="Screen Shot 2020-07-21 at 10 03 20 AM" src="https://user-images.githubusercontent.com/487433/88084548-71bce280-cb39-11ea-9f62-5acb4f71bc6b.png">
- Capitalized and bolded
<img width="683" alt="Screen Shot 2020-07-21 at 10 02 44 AM" src="https://user-images.githubusercontent.com/487433/88084553-72557900-cb39-11ea-879d-9ec61cf242df.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] Steejay edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
Steejay edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660151394
@mistercrunch see north star design for _add chart to new dashboard_
For the After image: The text field under radio buttons would benefit by adding a title so that its clear what the field is for. See SIP-34 screenshot you posted. Also I think "Save Chart" is more direct than "Save a Chart". "Save a Chart" implies unnecessary ambiguity in the statement.
<img width="1036" alt="Screen Shot 2020-07-17 at 7 50 44 AM" src="https://user-images.githubusercontent.com/60786102/87799581-40bf7380-c802-11ea-868a-2fcc5e221571.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] Steejay commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661987877
I prefer option 1. All caps is a little too aggressive/dominating. Additionally, the header should read "Save Chart" not "Save A Chart". Can we add the title "Chart Name*" above the text field as well?
see comments in https://github.com/apache/incubator-superset/pull/10355#issuecomment-660151394
----------------------------------------------------------------
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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **decrease** coverage by `4.06%`.
> The diff coverage is `95.65%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
- Coverage 69.70% 65.64% -4.07%
==========================================
Files 196 601 +405
Lines 18950 32235 +13285
Branches 0 3265 +3265
==========================================
+ Hits 13210 21162 +7952
- Misses 5740 10889 +5149
- Partials 0 184 +184
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.51% <94.73%> (?)` | |
| #python | `69.95% <100.00%> (+0.24%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `90.76% <94.73%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.68% <100.00%> (+0.29%)` | :arrow_up: |
| [...perset-frontend/src/components/AlteredSliceTag.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQWx0ZXJlZFNsaWNlVGFnLmpzeA==) | `98.82% <0.00%> (ø)` | |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (ø)` | |
| [...tend/src/dashboard/util/getDirectPathToTabIndex.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERpcmVjdFBhdGhUb1RhYkluZGV4Lmpz) | `100.00% <0.00%> (ø)` | |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <0.00%> (ø)` | |
| [...nd/src/dashboard/util/getFilterValuesByFilterId.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclZhbHVlc0J5RmlsdGVySWQuanM=) | `0.00% <0.00%> (ø)` | |
| [...dashboard/components/FilterIndicatorsContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvcnNDb250YWluZXIuanN4) | `69.35% <0.00%> (ø)` | |
| [superset-frontend/src/CRUD/utils.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvdXRpbHMuanM=) | `0.00% <0.00%> (ø)` | |
| ... and [409 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660559182
@Steejay , I decided to use the "CreatableSelect" component feature from react-select to avoid having to build the select box.
<img width="744" alt="Screen Shot 2020-07-18 at 4 47 53 PM" src="https://user-images.githubusercontent.com/487433/87863954-8ec59b80-c916-11ea-8860-1409d6bcca36.png">
I also improved the e2e cypress tests quite a bit, fixed and re-enabled tests that had been disabled.
----------------------------------------------------------------
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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661985391
Proposing two options:
<img width="674" alt="Screen Shot 2020-07-21 at 10 03 20 AM" src="https://user-images.githubusercontent.com/487433/88084548-71bce280-cb39-11ea-9f62-5acb4f71bc6b.png">
<img width="683" alt="Screen Shot 2020-07-21 at 10 02 44 AM" src="https://user-images.githubusercontent.com/487433/88084553-72557900-cb39-11ea-879d-9ec61cf242df.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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-662034880
Latest:
<img width="679" alt="Screen Shot 2020-07-21 at 11 33 58 AM" src="https://user-images.githubusercontent.com/487433/88092906-16452180-cb46-11ea-9051-8a555992e2eb.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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
=======================================
Coverage 69.70% 69.71%
=======================================
Files 196 196
Lines 18950 18952 +2
=======================================
+ Hits 13210 13212 +2
Misses 5740 5740
```
| Flag | Coverage Δ | |
|---|---|---|
| #python | `69.71% <100.00%> (+<0.01%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.44% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-662704391
Latest, with the new control-label
<img width="674" alt="Screen Shot 2020-07-22 at 2 23 15 PM" src="https://user-images.githubusercontent.com/487433/88230249-e7eb4300-cc26-11ea-9022-828aaa42e768.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 edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661968846
@ktmud I don't have data to back this up, but the "save AND create a new dashboard" is not super common, it's still very much possible and discoverable IMHO.
I think the previous modal looks awful and is super confusing. I'd hate to shelf this because it's not 100% SIP-34. This PR also re-enables/improves many unit tests and removes a lot of unneeded complexity.
@ktmud if the concern is the "create dashboard" is not discoverable enough, I can think of ways to improve that, like bolding the words "Create" and "Select" in the placeholder of the dashboard dropdown
----------------------------------------------------------------
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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-662857664
This design is definitely much cleaner than the existing layout. I'm OK with merging as is but would love to see the new iteration soon. A placeholder text still doesn't look as discoverable as the inline input in SIP-34 or the radio buttons in old design.
Another problem with current `CreatableSelect` is that it blocks users from creating a new dashboard with the same name as some existing dashboards.
----------------------------------------------------------------
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] rusackas commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661473074
> @Steejay , I decided to use the "CreatableSelect" component feature from react-select to avoid having to build the select box.
> <img alt="Screen Shot 2020-07-18 at 4 47 53 PM" width="744" src="https://user-images.githubusercontent.com/487433/87863954-8ec59b80-c916-11ea-8860-1409d6bcca36.png">
>
> <img alt="Screen Shot 2020-07-18 at 4 51 03 PM" width="673" src="https://user-images.githubusercontent.com/487433/87863978-ecf27e80-c916-11ea-9164-db75428487b3.png">
>
> I also improved the e2e cypress tests quite a bit, fixed and re-enabled tests that had been disabled.
The more I look at this the more I'm agonizing over the `🔘 Save (overwrite) 🔘 Save As` choice.
Could it just be:
```
Save as:
[SomeChartame ]
⚠️ Warning: This will overwrite the current *SomeChartame* chart
```
^ Just displaying an overwrite warning, rather than forcing a choice?
----------------------------------------------------------------
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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **decrease** coverage by `4.28%`.
> The diff coverage is `95.83%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
- Coverage 69.70% 65.42% -4.29%
==========================================
Files 196 603 +407
Lines 18950 32393 +13443
Branches 0 3287 +3287
==========================================
+ Hits 13210 21194 +7984
- Misses 5740 11015 +5275
- Partials 0 184 +184
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.26% <95.00%> (?)` | |
| #python | `69.75% <100.00%> (+0.04%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `90.90% <95.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.58% <100.00%> (+0.18%)` | :arrow_up: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.96% <0.00%> (-12.98%)` | :arrow_down: |
| [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | `93.75% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/exceptions.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `95.12% <0.00%> (-4.88%)` | :arrow_down: |
| [superset/views/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi5weQ==) | `58.62% <0.00%> (-3.96%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `91.12% <0.00%> (-1.62%)` | :arrow_down: |
| [superset/views/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `72.50% <0.00%> (-0.92%)` | :arrow_down: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.04% <0.00%> (-0.34%)` | :arrow_down: |
| [superset/app.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXBwLnB5) | `80.95% <0.00%> (-0.08%)` | :arrow_down: |
| ... and [424 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...32b0f52](https://codecov.io/gh/apache/incubator-superset/pull/10355?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] codecov-commenter edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **increase** coverage by `0.93%`.
> The diff coverage is `95.65%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
+ Coverage 69.70% 70.64% +0.93%
==========================================
Files 196 601 +405
Lines 18950 32236 +13286
Branches 0 3265 +3265
==========================================
+ Hits 13210 22773 +9563
- Misses 5740 9357 +3617
- Partials 0 106 +106
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `55.68% <94.73%> (?)` | |
| #javascript | `59.51% <94.73%> (?)` | |
| #python | `69.95% <100.00%> (+0.24%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `92.30% <94.73%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.68% <100.00%> (+0.29%)` | :arrow_up: |
| [...rset-frontend/src/SqlLab/components/QueryTable.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5VGFibGUuanN4) | `61.53% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/util/resizableConfig.ts](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3Jlc2l6YWJsZUNvbmZpZy50cw==) | `100.00% <0.00%> (ø)` | |
| [...c/dashboard/components/dnd/AddSliceDragPreview.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9BZGRTbGljZURyYWdQcmV2aWV3LmpzeA==) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/components/Pagination.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnaW5hdGlvbi50c3g=) | `86.20% <0.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `62.60% <0.00%> (ø)` | |
| [...nd/src/messageToasts/containers/ToastPresenter.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvY29udGFpbmVycy9Ub2FzdFByZXNlbnRlci5qc3g=) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/components/Select/index.ts](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L2luZGV4LnRz) | `100.00% <0.00%> (ø)` | |
| [...rc/dashboard/components/FilterIndicatorTooltip.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvclRvb2x0aXAuanN4) | `100.00% <0.00%> (ø)` | |
| ... and [409 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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 merged pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355
----------------------------------------------------------------
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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660559182
@Steejay , I decided to use the "CreatableSelect" component feature from react-select to avoid having to build the select box.
<img width="744" alt="Screen Shot 2020-07-18 at 4 47 53 PM" src="https://user-images.githubusercontent.com/487433/87863954-8ec59b80-c916-11ea-8860-1409d6bcca36.png">
<img width="673" alt="Screen Shot 2020-07-18 at 4 51 03 PM" src="https://user-images.githubusercontent.com/487433/87863978-ecf27e80-c916-11ea-9164-db75428487b3.png">
I also improved the e2e cypress tests quite a bit, fixed and re-enabled tests that had been disabled.
----------------------------------------------------------------
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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **decrease** coverage by `4.06%`.
> The diff coverage is `95.65%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
- Coverage 69.70% 65.64% -4.07%
==========================================
Files 196 601 +405
Lines 18950 32235 +13285
Branches 0 3265 +3265
==========================================
+ Hits 13210 21162 +7952
- Misses 5740 10889 +5149
- Partials 0 184 +184
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.51% <94.73%> (?)` | |
| #python | `69.95% <100.00%> (+0.24%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `90.76% <94.73%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.68% <100.00%> (+0.29%)` | :arrow_up: |
| [...tend/src/SqlLab/components/ScheduleQueryButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NjaGVkdWxlUXVlcnlCdXR0b24uanN4) | `8.62% <0.00%> (ø)` | |
| [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `74.07% <0.00%> (ø)` | |
| [...frontend/src/components/ListView/LegacyFilters.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGVnYWN5RmlsdGVycy50c3g=) | `75.60% <0.00%> (ø)` | |
| [...perset-frontend/src/dashboard/util/isValidChild.ts](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2lzVmFsaWRDaGlsZC50cw==) | `86.66% <0.00%> (ø)` | |
| [...rontend/src/explore/components/EmbedCodeButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FbWJlZENvZGVCdXR0b24uanN4) | `70.00% <0.00%> (ø)` | |
| [...rset-frontend/src/explore/controlPanels/DeckArc.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy9EZWNrQXJjLmpz) | `0.00% <0.00%> (ø)` | |
| [...t-frontend/src/SqlLab/components/ColumnElement.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0NvbHVtbkVsZW1lbnQudHN4) | `100.00% <0.00%> (ø)` | |
| [...shboard/components/filterscope/FilterFieldItem.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2ZpbHRlcnNjb3BlL0ZpbHRlckZpZWxkSXRlbS5qc3g=) | `50.00% <0.00%> (ø)` | |
| ... and [409 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661968846
@ktmud I don't have data to back this up, but the "save AND create a new dashboard" is not super common, it's still very much possible and discoverable IMHO. I think the previous modal looks awful and is super confusing.
I'd hate to shelf this because it's not 100% SIP-34. This PR also re-enables/improves many unit tests and removes a lot of unneeded complexity.
@ktmud if the concern is the "create dashboard" is not discoverable enough, I can think of ways to improve that, like bolding the words "Create" and "Select" in the placeholder of the dashboard dropdown
----------------------------------------------------------------
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 #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **increase** coverage by `0.77%`.
> The diff coverage is `95.83%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
+ Coverage 69.70% 70.48% +0.77%
==========================================
Files 196 603 +407
Lines 18950 32385 +13435
Branches 0 3287 +3287
==========================================
+ Hits 13210 22826 +9616
- Misses 5740 9455 +3715
- Partials 0 104 +104
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `55.56% <95.00%> (?)` | |
| #javascript | `59.26% <95.00%> (?)` | |
| #python | `69.77% <100.00%> (+0.06%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `92.42% <95.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.58% <100.00%> (+0.18%)` | :arrow_up: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.96% <0.00%> (-12.98%)` | :arrow_down: |
| [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | `93.54% <0.00%> (-6.46%)` | :arrow_down: |
| [superset/exceptions.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `95.12% <0.00%> (-4.88%)` | :arrow_down: |
| [superset/views/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3Mvc3FsX2xhYi5weQ==) | `58.62% <0.00%> (-3.96%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `91.12% <0.00%> (-1.62%)` | :arrow_down: |
| [superset/views/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `72.50% <0.00%> (-0.92%)` | :arrow_down: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.04% <0.00%> (-0.34%)` | :arrow_down: |
| [superset/app.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXBwLnB5) | `80.95% <0.00%> (-0.08%)` | :arrow_down: |
| ... and [421 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...32b0f52](https://codecov.io/gh/apache/incubator-superset/pull/10355?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] codecov-commenter edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **decrease** coverage by `4.06%`.
> The diff coverage is `95.65%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
- Coverage 69.70% 65.64% -4.07%
==========================================
Files 196 601 +405
Lines 18950 32235 +13285
Branches 0 3265 +3265
==========================================
+ Hits 13210 21161 +7951
- Misses 5740 10890 +5150
- Partials 0 184 +184
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.51% <94.73%> (?)` | |
| #python | `69.94% <100.00%> (+0.23%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `90.76% <94.73%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.68% <100.00%> (+0.29%)` | :arrow_up: |
| [...e/controlPanels/timeGrainSqlaAnimationOverrides.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy90aW1lR3JhaW5TcWxhQW5pbWF0aW9uT3ZlcnJpZGVzLmpz) | `0.00% <0.00%> (ø)` | |
| [...ntend/src/dashboard/components/FilterIndicator.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvci5qc3g=) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/components/Menu/Menu.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9NZW51LmpzeA==) | `88.23% <0.00%> (ø)` | |
| [superset-frontend/src/utils/hostNamesConfig.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2hvc3ROYW1lc0NvbmZpZy5qcw==) | `37.50% <0.00%> (ø)` | |
| [...-frontend/src/explore/controlPanels/DeckGeojson.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbFBhbmVscy9EZWNrR2VvanNvbi5qcw==) | `0.00% <0.00%> (ø)` | |
| [...oard/components/gridComponents/new/NewMarkdown.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL25ldy9OZXdNYXJrZG93bi5qc3g=) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/utils/reducerUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL3JlZHVjZXJVdGlscy5qcw==) | `0.00% <0.00%> (ø)` | |
| ... and [408 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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] codecov-commenter edited a comment on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **decrease** coverage by `4.20%`.
> The diff coverage is `95.65%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
==========================================
- Coverage 69.70% 65.50% -4.21%
==========================================
Files 196 601 +405
Lines 18950 32235 +13285
Branches 0 3265 +3265
==========================================
+ Hits 13210 21117 +7907
- Misses 5740 10934 +5194
- Partials 0 184 +184
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.51% <94.73%> (?)` | |
| #python | `69.71% <100.00%> (+<0.01%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset-frontend/src/explore/components/SaveModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9TYXZlTW9kYWwuanN4) | `90.76% <94.73%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.44% <100.00%> (+0.04%)` | :arrow_up: |
| [.../src/dashboard/components/gridComponents/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `65.59% <0.00%> (ø)` | |
| [superset-frontend/src/welcome/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3dlbGNvbWUvQXBwLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...uperset-frontend/src/components/ExpandableList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXhwYW5kYWJsZUxpc3QudHN4) | `88.88% <0.00%> (ø)` | |
| [superset-frontend/src/logger/LogUtils.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2xvZ2dlci9Mb2dVdGlscy5qcw==) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/components/TableSelector.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci5qc3g=) | `80.00% <0.00%> (ø)` | |
| [...essageToasts/utils/getToastsFromPyFlashMessages.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvdXRpbHMvZ2V0VG9hc3RzRnJvbVB5Rmxhc2hNZXNzYWdlcy5qcw==) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/components/FormRow.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybVJvdy5qc3g=) | `92.30% <0.00%> (ø)` | |
| [...-frontend/src/explore/reducers/saveModalReducer.js](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvc2F2ZU1vZGFsUmVkdWNlci5qcw==) | `0.00% <0.00%> (ø)` | |
| ... and [397 more](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...196a73b](https://codecov.io/gh/apache/incubator-superset/pull/10355?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] rusackas commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-679292372
Impacts #8976
----------------------------------------------------------------
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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660562757
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=h1) Report
> Merging [#10355](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1a41ea49880520d296a7cb58f2d8ebccfbd5681a&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10355?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10355 +/- ##
=======================================
Coverage 69.70% 69.71%
=======================================
Files 196 196
Lines 18950 18952 +2
=======================================
+ Hits 13210 13212 +2
Misses 5740 5740
```
| Flag | Coverage Δ | |
|---|---|---|
| #python | `69.71% <100.00%> (+<0.01%)` | :arrow_up: |
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/10355?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10355/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.44% <100.00%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10355?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/10355?src=pr&el=footer). Last update [1a41ea4...d02f208](https://codecov.io/gh/apache/incubator-superset/pull/10355?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] Steejay commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-660151394
@mistercrunch see north star design for _add chart to new dashboard_
For the After image: The text field under radio buttons would benefit by adding a title so that its clear what the field is for. See SIP-34 screenshot you posted.
<img width="1036" alt="Screen Shot 2020-07-17 at 7 50 44 AM" src="https://user-images.githubusercontent.com/60786102/87799581-40bf7380-c802-11ea-868a-2fcc5e221571.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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-662045279
@ktmud I'd love your blessing on this, but I think this is solid mergeable progress and a good checkpoint in the right direction
----------------------------------------------------------------
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 commented on pull request #10355: feat: SIP-34 explore save modal
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10355:
URL: https://github.com/apache/incubator-superset/pull/10355#issuecomment-661494548
>
> ```
> Save as:
> [SomeChartame ]
> ⚠️ Warning: This will overwrite the current *SomeChartame* chart
> ```
>
> ^ Just displaying an overwrite warning, rather than forcing a choice?
I don't think this works as two charts can have the same title. Sometimes users just want to save a new copy of a chart and play with it.
I also feel `CreatableSelect` is not quite clear as the original radio buttons. Maybe keep it as is before we implement the SIP-34 design?
----------------------------------------------------------------
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