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 2021/02/08 18:02:20 UTC
[GitHub] [superset] AAfghahi opened a new pull request #12923: fix: added text and changed margins
AAfghahi opened a new pull request #12923:
URL: https://github.com/apache/superset/pull/12923
### SUMMARY
Also changed the form-group to have margin 8px for all form groups.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:https://github.com/apache/superset/issues/10825
----------------------------------------------------------------
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] [superset] AAfghahi commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772973288
Hello,
No need to apologize! I was reading it too literally.
Anyway, I needed to target the help span rather than the group. The newest
push accounts for it-
[image: Screen Shot 2021-02-03 at 9.15.33 PM.png]
[image: Screen Shot 2021-02-03 at 9.08.35 PM.png]
On Wed, Feb 3, 2021 at 7:27 PM Stephen Jordan <no...@github.com>
wrote:
> @AAfghahi <https://github.com/AAfghahi> sorry about the miscommunication.
> shouldve been more clear here. i was referring to the form groups on this
> page.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/superset/pull/12923#issuecomment-772930205>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ALVKTWG3PQVJTGUZJISJOUDS5HSWBANCNFSM4XBLSHEQ>
> .
>
--
Arash Afghahi
----------------------------------------------------------------
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] [superset] AAfghahi commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r569832554
##########
File path: superset-frontend/stylesheets/less/cosmo/variables.less
##########
@@ -220,7 +220,7 @@
);
// ** `.form-group` margin
-@form-group-margin-bottom: 16px;
+@form-group-margin-bottom: 8px;
Review comment:
Sorry for taking so long, Beto was going through the backend architecture. I just made this change.
----------------------------------------------------------------
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] [superset] eschutho commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-773530259
@AAfghahi can you repaste the new screenshots? looks like they're not showing up. thanks!
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (6f2d420) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `2.11%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 66.87% -2.12%
==========================================
Files 1025 489 -536
Lines 48765 28661 -20104
Branches 5188 0 -5188
==========================================
- Hits 33639 19166 -14473
+ Misses 14992 9495 -5497
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.87% <ø> (-0.48%)` | :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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.85% <0.00%> (-6.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-6.07%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| ... and [553 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...ea868cc](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r570465964
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -51,13 +51,17 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
const DatasourceContainer = styled.div`
.change-warning {
- margin: 16px 10px 0;
+ margin: 16px 10px 0x;
Review comment:
was this change intentional?
----------------------------------------------------------------
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] [superset] AAfghahi commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-773537111
Sorry about the screenshots, I replied via email, and that does not add them.
----------------------------------------------------------------
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] [superset] Steejay commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772930205
@AAfghahi sorry about the miscommunication. shouldve been more clear here. i was referring to the form groups on this page.
----------------------------------------------------------------
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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r569774522
##########
File path: superset-frontend/stylesheets/less/cosmo/variables.less
##########
@@ -220,7 +220,7 @@
);
// ** `.form-group` margin
-@form-group-margin-bottom: 16px;
+@form-group-margin-bottom: 8px;
Review comment:
we're going to be removing the .less styling, and also so that we don't change things too globally, you can instead add some styling for this selector with the emotion styles in `DatasourceEditor.jsx` under `const DatasourceContainer = styled.div`
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (6f2d420) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `8.25%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 60.73% -8.26%
==========================================
Files 1025 966 -59
Lines 48765 45887 -2878
Branches 5188 4444 -744
==========================================
- Hits 33639 27868 -5771
- Misses 14992 18019 +3027
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `49.79% <ø> (-1.04%)` | :arrow_down: |
| javascript | `?` | |
| python | `67.29% <ø> (-0.05%)` | :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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/welcome/Welcome.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9XZWxjb21lLnRzeA==) | `2.94% <0.00%> (-85.95%)` | :arrow_down: |
| ... and [413 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...ea868cc](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (07d05ed) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `2.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 66.96% -2.02%
==========================================
Files 1025 489 -536
Lines 48765 28714 -20051
Branches 5188 0 -5188
==========================================
- Hits 33639 19228 -14411
+ Misses 14992 9486 -5506
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.96% <ø> (-0.39%)` | :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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.51% <0.00%> (-5.91%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `88.30% <0.00%> (-3.63%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.05% <0.00%> (-2.72%)` | :arrow_down: |
| ... and [550 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...07d05ed](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r570465964
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -51,13 +51,17 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
const DatasourceContainer = styled.div`
.change-warning {
- margin: 16px 10px 0;
+ margin: 16px 10px 0x;
Review comment:
was this change intentional?
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -58,6 +58,10 @@ const DatasourceContainer = styled.div`
.change-warning .bold {
font-weight: ${({ theme }) => theme.typography.weights.bold};
}
+
+ .help-block {
+ margin-top: 8px;
+ }
Review comment:
when I render this, I still see the margin-bottom of the form-group, so it's still making a total of 16px.
This is a bit tricky actually. I read this as
form-group should always have a margin bottom of 8px
when there are two siblings (form-group followed by a form-group), then the second one should have a margin-top of 16px.
I think if you go that way, you won't need to add anything additional to the help-block.
lmk if that helps
----------------------------------------------------------------
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] [superset] hughhhh merged pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
hughhhh merged pull request #12923:
URL: https://github.com/apache/superset/pull/12923
----------------------------------------------------------------
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] [superset] AAfghahi commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r570472984
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -51,13 +51,17 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
const DatasourceContainer = styled.div`
.change-warning {
- margin: 16px 10px 0;
+ margin: 16px 10px 0x;
Review comment:
No, and I noticed that there was one test that I failed, and maybe this is why. Thank you for noticing 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] [superset] codecov-io commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (6ee5f8e) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `7.14%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 61.83% -7.15%
==========================================
Files 1025 536 -489
Lines 48765 20075 -28690
Branches 5188 5241 +53
==========================================
- Hits 33639 12414 -21225
+ Misses 14992 7449 -7543
- Partials 134 212 +78
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.83% <ø> (ø)` | |
| python | `?` | |
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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `66.95% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12923/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/superset/pull/12923/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/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [672 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...6ee5f8e](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] hughhhh closed pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
hughhhh closed pull request #12923:
URL: https://github.com/apache/superset/pull/12923
----------------------------------------------------------------
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] [superset] Steejay commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772929191
> I think the margins above the titles should stay at what they were before (16px?) and just the margin between the input field and the help text should be 8px, but @Steejay can confirm!
Yes confirmed cc @yousoph @AAfghahi
----------------------------------------------------------------
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] [superset] yousoph commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
yousoph commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772914967
I think the margins above the titles should stay at what they were before (16px?) and just the margin between the input field and the help text should be 8px, but @steejay can confirm!
----------------------------------------------------------------
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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r570665468
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -58,6 +58,10 @@ const DatasourceContainer = styled.div`
.change-warning .bold {
font-weight: ${({ theme }) => theme.typography.weights.bold};
}
+
+ .help-block {
+ margin-top: 8px;
+ }
Review comment:
when I render this, I still see the margin-bottom of the form-group, so it's still making a total of 16px.
This is a bit tricky actually. I read this as
form-group should always have a margin bottom of 8px
when there are two siblings (form-group followed by a form-group), then the second one should have a margin-top of 16px.
I think if you go that way, you won't need to add anything additional to the help-block.
lmk if that helps
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
----------------------------------------------------------------
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] [superset] eschutho commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-773530259
@AAfghahi can you repaste the new screenshots? looks like they're not showing up. thanks!
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (6f2d420) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `8.51%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 60.46% -8.52%
==========================================
Files 1025 966 -59
Lines 48765 45879 -2886
Branches 5188 4444 -744
==========================================
- Hits 33639 27740 -5899
- Misses 14992 18139 +3147
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `49.79% <ø> (-1.04%)` | :arrow_down: |
| javascript | `?` | |
| python | `66.87% <ø> (-0.48%)` | :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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/welcome/Welcome.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9XZWxjb21lLnRzeA==) | `2.94% <0.00%> (-85.95%)` | :arrow_down: |
| ... and [423 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...ea868cc](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (3c9c4b6) into [master](https://codecov.io/gh/apache/superset/commit/3235d918aad2e82be3d52aeb51955e3854a10764?el=desc) (3235d91) will **decrease** coverage by `16.82%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
===========================================
- Coverage 69.79% 52.97% -16.83%
===========================================
Files 1026 480 -546
Lines 48865 17315 -31550
Branches 5261 4485 -776
===========================================
- Hits 34106 9172 -24934
+ Misses 14634 8143 -6491
+ Partials 125 0 -125
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.97% <ø> (-0.01%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/welcome/Welcome.tsx](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9XZWxjb21lLnRzeA==) | `2.94% <0.00%> (-85.95%)` | :arrow_down: |
| ... and [882 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [3235d91...3c9c4b6](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] AAfghahi commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r569777858
##########
File path: superset-frontend/stylesheets/less/cosmo/variables.less
##########
@@ -220,7 +220,7 @@
);
// ** `.form-group` margin
-@form-group-margin-bottom: 16px;
+@form-group-margin-bottom: 8px;
Review comment:
Hello, I was also uncomfortable changing this, since it is a global change. In the ticket, Stephen mentioned that he wanted the change for all form groups. So I thought that it might be for everything. Relooking at it, he might have meant for all of the form groups on this page however.
Will go back and change it using emotion styles instead. It will be a good opportunity to work with emotion.
----------------------------------------------------------------
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] [superset] AAfghahi commented on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-773537111
Sorry about the screenshots, I replied via email, and that does not add them.
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12923:
URL: https://github.com/apache/superset/pull/12923#issuecomment-772815704
# [Codecov](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=h1) Report
> Merging [#12923](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=desc) (07d05ed) into [master](https://codecov.io/gh/apache/superset/commit/b5b0c2c8a2a4300aa6f7a0f1972047fee5dedd08?el=desc) (b5b0c2c) will **decrease** coverage by `2.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12923?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12923 +/- ##
==========================================
- Coverage 68.98% 66.96% -2.02%
==========================================
Files 1025 489 -536
Lines 48765 28714 -20051
Branches 5188 0 -5188
==========================================
- Hits 33639 19228 -14411
+ Misses 14992 9486 -5506
+ Partials 134 0 -134
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.96% <ø> (-0.39%)` | :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/superset/pull/12923?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.51% <0.00%> (-5.91%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `88.30% <0.00%> (-3.63%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.05% <0.00%> (-2.72%)` | :arrow_down: |
| ... and [550 more](https://codecov.io/gh/apache/superset/pull/12923/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12923?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/superset/pull/12923?src=pr&el=footer). Last update [b5b0c2c...07d05ed](https://codecov.io/gh/apache/superset/pull/12923?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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r569774522
##########
File path: superset-frontend/stylesheets/less/cosmo/variables.less
##########
@@ -220,7 +220,7 @@
);
// ** `.form-group` margin
-@form-group-margin-bottom: 16px;
+@form-group-margin-bottom: 8px;
Review comment:
we're going to be removing the .less styling, and also so that we don't change things too globally, you can add some styling for this selector with the emotion styles in `DatasourceEditor.jsx` under `const DatasourceContainer = styled.div`
----------------------------------------------------------------
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] [superset] AAfghahi commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r571101958
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -58,6 +58,10 @@ const DatasourceContainer = styled.div`
.change-warning .bold {
font-weight: ${({ theme }) => theme.typography.weights.bold};
}
+
+ .help-block {
+ margin-top: 8px;
+ }
Review comment:
Yeah that helps a lot, it really cleared up my thinking about it. I also ended up reading more about how margin works. I ended up trying something else, because nothing I was reading was helping me find a way of implementing your solution.
I ended up targeting the specific form group that is above each help block (form-group-md). I changed the margin-bottom of these to be 8px. However, in order to make sure that 'Extra' and 'Owners' section had a help block with a margin of 8px, I also added a margin-top to the help-block.
Originally I thought that a margin-bottom of 8 and a margin-top of 8 on two elements would make 16px, but they don't actually add the way that I thought they might. It simple goes to the highest margin value. Which is cool.
It looks like this now:
The margin between form groups is 16px:
The margin between help-block and form-group is 8px:
----------------------------------------------------------------
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] [superset] eschutho commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r572252436
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -58,6 +58,10 @@ const DatasourceContainer = styled.div`
.change-warning .bold {
font-weight: ${({ theme }) => theme.typography.weights.bold};
}
+
+ .help-block {
+ margin-top: 8px;
+ }
Review comment:
This look great. One other thing I was hinting at was sibling selectors: `when there are two siblings (form-group followed by a form-group), then the second one should have a margin-top of 16px.` So that would be `.form-group + .form-group` but it looks like you made it work this way, too.
----------------------------------------------------------------
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] [superset] AAfghahi commented on a change in pull request #12923: fix: added text and changed margins
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #12923:
URL: https://github.com/apache/superset/pull/12923#discussion_r570472984
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -51,13 +51,17 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
const DatasourceContainer = styled.div`
.change-warning {
- margin: 16px 10px 0;
+ margin: 16px 10px 0x;
Review comment:
No, and I noticed that there was one test that I failed, and maybe this is why. Thank you for noticing 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