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/11/03 13:36:31 UTC
[GitHub] [incubator-superset] kkucharc opened a new pull request #11525: fix: extended droppable area on the top of dashboards
kkucharc opened a new pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525
----------------------------------------------------------------
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-io commented on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-720477827
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11525?src=pr&el=h1) Report
> Merging [#11525](https://codecov.io/gh/apache/incubator-superset/pull/11525?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5d9448f3fe5999568cf7fd46a211947f48b7cd7e?el=desc) will **decrease** coverage by `5.49%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11525?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11525 +/- ##
==========================================
- Coverage 66.58% 61.08% -5.50%
==========================================
Files 873 420 -453
Lines 41767 26133 -15634
Branches 3827 0 -3827
==========================================
- Hits 27809 15964 -11845
+ Misses 13856 10169 -3687
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `61.08% <ø> (-1.01%)` | :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/11525?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.53%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `81.94% <0.00%> (-6.02%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| ... and [470 more](https://codecov.io/gh/apache/incubator-superset/pull/11525/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11525?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/11525?src=pr&el=footer). Last update [5d9448f...028efba](https://codecov.io/gh/apache/incubator-superset/pull/11525?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 a change in pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#discussion_r517519769
##########
File path: superset-frontend/src/dashboard/stylesheets/dnd.less
##########
@@ -123,7 +129,12 @@
}
& > .empty-droptarget:first-child {
- height: 24px;
+ height: 48px;
margin-top: -24px;
+ margin-bottom: -24px;
+ }
+
+ & > .empty-droptarget:only-child {
+ height: 80vh;
Review comment:
This `vh` unit might not be accurate on viewports that are very tall or very short. The header area is not a flexible 20%, but some fixed height I believe. You might be better off with something like `height: calc(100vh - XXpx)`
That said I'm LOVING this idea.
----------------------------------------------------------------
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] etr2460 commented on a change in pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#discussion_r516111384
##########
File path: superset-frontend/src/dashboard/stylesheets/dnd.less
##########
@@ -123,7 +129,8 @@
}
& > .empty-droptarget:first-child {
- height: 24px;
+ height: 48px;
Review comment:
any reason why not to make this the entire size of the dashboard when it's empty?
----------------------------------------------------------------
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] kkucharc commented on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
kkucharc commented on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-721775806
Hi @etr2460 @rusackas , I added commit that makes whole dashboard area available when it's empty. Can you take a look if it's better this way:
----------------------------------------------------------------
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-io edited a comment on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-720477827
----------------------------------------------------------------
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 #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525
----------------------------------------------------------------
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] kkucharc commented on a change in pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#discussion_r516528654
##########
File path: superset-frontend/src/dashboard/stylesheets/dnd.less
##########
@@ -123,7 +129,8 @@
}
& > .empty-droptarget:first-child {
- height: 24px;
+ height: 48px;
Review comment:
This is really good idea. But unfortunately it might not be that easy, because we would have to detect if first row is placed and dynamically change height of the `droptarget` element to limit back the area above first row.
----------------------------------------------------------------
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] kkucharc closed pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
kkucharc closed pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525
----------------------------------------------------------------
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] villebro commented on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-721781947
@kkucharc I love this! 💯
----------------------------------------------------------------
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] junlincc edited a comment on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-724267087
just tested. the experience of adding components to new dashboard first row is muuuuuch better thanks to the fix!
however, the general experience of adding components to a dashboard is still VERY unintuitive to users. We should consider a bigger fix post 1.0. @suddjian
----------------------------------------------------------------
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] junlincc commented on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-724267087
just tested. the experience of adding components to new dashboard first row is muuuuuch better thanks to the fix!
however, the general experience of adding components to a dashboard is still VERY unintuitive to users. We should consider a bigger fix post 1.0. @suddjian
----------------------------------------------------------------
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] kkucharc commented on a change in pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#discussion_r517525898
##########
File path: superset-frontend/src/dashboard/stylesheets/dnd.less
##########
@@ -123,7 +129,12 @@
}
& > .empty-droptarget:first-child {
- height: 24px;
+ height: 48px;
margin-top: -24px;
+ margin-bottom: -24px;
+ }
+
+ & > .empty-droptarget:only-child {
+ height: 80vh;
Review comment:
Thanks a lot! I'm also concerned about this `vh`. On the other hand top hasn't defined size and in mobile view it becomes really big. But I will try figure something out.
----------------------------------------------------------------
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] junlincc commented on pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#issuecomment-721257314
Thank you so much Kashia!!
----------------------------------------------------------------
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 a change in pull request #11525: fix: extended droppable area on the top of dashboards
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11525:
URL: https://github.com/apache/incubator-superset/pull/11525#discussion_r517528673
##########
File path: superset-frontend/src/dashboard/stylesheets/dnd.less
##########
@@ -123,7 +129,12 @@
}
& > .empty-droptarget:first-child {
- height: 24px;
+ height: 48px;
margin-top: -24px;
+ margin-bottom: -24px;
+ }
+
+ & > .empty-droptarget:only-child {
+ height: 80vh;
Review comment:
Not sure if it would work (depends on the positioning of its ancestors), but `position:absolute` with top/bottom/left/right of 0 might work.
----------------------------------------------------------------
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