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 2022/04/06 22:23:34 UTC
[GitHub] [superset] etr2460 opened a new pull request, #19579: fix: missing home label on welcome page
etr2460 opened a new pull request, #19579:
URL: https://github.com/apache/superset/pull/19579
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
Fixes the following bug where the "Home" text wasn't displayed on the welcome page:
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After:
### TESTING INSTRUCTIONS
test env soon
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
to: @ktmud @michael-s-molina @rusackas
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] geido commented on a diff in pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
geido commented on code in PR #19579:
URL: https://github.com/apache/superset/pull/19579#discussion_r845020734
##########
superset-frontend/src/assets/stylesheets/superset.less:
##########
@@ -545,7 +545,6 @@ td.filtered {
display: flex;
flex-direction: column;
justify-content: center;
- display: none;
}
Review Comment:
```suggestion
/* hides the logo while the page loads. Emotion will take care of displaying */
header a.navbar-brand {
display: none;
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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[bot] commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1090874334
# [Codecov](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19579](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c1de48) into [master](https://codecov.io/gh/apache/superset/commit/444b4f5832a8b29a8410b315731f3e86ce2984ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (444b4f5) will **decrease** coverage by `0.19%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #19579 +/- ##
==========================================
- Coverage 53.94% 53.74% -0.20%
==========================================
Files 1680 1680
Lines 64271 64271
Branches 6564 6564
==========================================
- Hits 34670 34543 -127
- Misses 27899 28026 +127
Partials 1702 1702
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| presto | `52.56% <ø> (ø)` | |
| python | `56.10% <ø> (-0.41%)` | :arrow_down: |
| unit | `47.21% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `35.63% <0.00%> (-28.74%)` | :arrow_down: |
| [superset/models/sql\_types/base.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF90eXBlcy9iYXNlLnB5) | `36.84% <0.00%> (-15.79%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `71.77% <0.00%> (-6.46%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `79.34% <0.00%> (-3.27%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `71.38% <0.00%> (-1.34%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `76.20% <0.00%> (-0.56%)` | :arrow_down: |
| [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/19579/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `59.44% <0.00%> (-0.12%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [444b4f5...8c1de48](https://codecov.io/gh/apache/superset/pull/19579?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] geido commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
geido commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1091652973
@etr2460 this is the PR https://github.com/apache/superset/pull/19586. It fixes the problem described here while enhancing the styles to match our guidelines. Thanks again!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] geido commented on a diff in pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
geido commented on code in PR #19579:
URL: https://github.com/apache/superset/pull/19579#discussion_r845020734
##########
superset-frontend/src/assets/stylesheets/superset.less:
##########
@@ -545,7 +545,6 @@ td.filtered {
display: flex;
flex-direction: column;
justify-content: center;
- display: none;
}
Review Comment:
```suggestion
/* hides the logo while the page loads. Emotion will take care of displaying */
header a.navbar-brand {
display: none;
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] etr2460 commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
etr2460 commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1095569173
closing in favor of #19586
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] geido commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
geido commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1091624664
> oh wait, i see that @geido just made this change recently. can you provide more context here?
@etr2460 thanks for catching this!
It was an emergency change because this stylesheet was taking precedence over the Emotion styles causing the logo to resize on page load. In order to avoid moving the styles back to less, we decided to display none here and let Emotion display correctly on page loaded. However, we clearly overlooked the change for the rush.
I made a suggested change to your code so that the display none is only applied to the right element and does not affect the others using the same class, as the "Home" text.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] etr2460 closed pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
etr2460 closed pull request #19579: fix: missing home label on welcome page
URL: https://github.com/apache/superset/pull/19579
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] etr2460 commented on a diff in pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
etr2460 commented on code in PR #19579:
URL: https://github.com/apache/superset/pull/19579#discussion_r844469538
##########
superset-frontend/src/assets/stylesheets/superset.less:
##########
@@ -545,7 +545,6 @@ td.filtered {
display: flex;
flex-direction: column;
justify-content: center;
- display: none;
Review Comment:
display flex is set in this class already, why are we adding display none here??? this seems obviously wrong, but i'll test in a test env before merge
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] etr2460 commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
etr2460 commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1090872971
oh wait, i see that @geido just made this change recently. can you provide more context here?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] geido commented on pull request #19579: fix: missing home label on welcome page
Posted by GitBox <gi...@apache.org>.
geido commented on PR #19579:
URL: https://github.com/apache/superset/pull/19579#issuecomment-1091599252
> oh wait, i see that @geido just made this change recently. can you provide more context here?
@etr2460 there must have been some merging mistake here. The idea was to replace the flex with display none as this stylesheet is loaded earlier than the emotion styles, which was causing the logo to change in size while the page was loading. The emotion styles would take care of overriding the display none with the flex. However, this shouldn't affect the "Home" text which seems to be the case here. Let me have a deeper look and see what happened.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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