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/02/01 13:24:07 UTC
[GitHub] [superset] villebro opened a new pull request #18246: fix(listview): add nowrap to view mode container
villebro opened a new pull request #18246:
URL: https://github.com/apache/superset/pull/18246
### SUMMARY
Add `white-space: nowrap;` to the list view view mode container to prevent wrapping.
### AFTER
### BEFORE
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually 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:
- [ ] Required feature flags:
- [ ] 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
--
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] villebro commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1026875359
> @villebro In this case I am not sure if we should do a no-wrap. Those view buttons side by side take up a lot of space and it's even more obvious when you have a few rows of filters. We could actually keep this logic that if there is more than one row the icons are stacked on the top of each other or improve the margins/spacings of the whole component + icons. Wdyt?
>
> The view icon could also be a drop down radio button to save the space.
Thanks @kasiazjc for the review! In the current design, keeping the no-wrap kinda feels ok as the dropdowns usually aren't able to gobble up the whole available space on the right (=the no-wrapped buttons don't eat up valuable horizontal space). However, having said that, I think an alternative layout in the whole component that adjusts to different sizes better would probably be optimal. Did you have a particular layout in mind?
--
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] kasiazjc commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
kasiazjc commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1026943061
@villebro Makes sense! So I thought about something like this.
The popover logic is something that we are implementing now for time columns in explore with @kgabryje. This way we would be able to save a little bit of precious space, let me know what you think (and do not mind the mocks, i found random ones in figma).
--
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] kasiazjc commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
kasiazjc commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1026850818
@villebro In this case I am not sure if we should do a no-wrap. Those view buttons side by side take up a lot of space and it's even more obvious when you have a few rows of filters. We could actually keep this logic that if there is more than one row the icons are stacked on the top of each other or improve the margins/spacings of the whole component + icons. Wdyt?
The view icon could also be a drop down radio button to save the space.
--
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] villebro commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1027100816
I like this @kasiazjc π @rusackas @mistercrunch thoughts on rolling out this new design 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] codecov[bot] commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1026879225
# [Codecov](https://codecov.io/gh/apache/superset/pull/18246?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 [#18246](https://codecov.io/gh/apache/superset/pull/18246?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c541e8) into [master](https://codecov.io/gh/apache/superset/commit/c40b337978717b149984ed00c5359cf2c2394254?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c40b337) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/18246?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18246 +/- ##
=======================================
Coverage 66.30% 66.30%
=======================================
Files 1592 1592
Lines 62437 62439 +2
Branches 6292 6295 +3
=======================================
+ Hits 41401 41403 +2
Misses 19383 19383
Partials 1653 1653
```
| Flag | Coverage Ξ | |
|---|---|---|
| javascript | `51.37% <ΓΈ> (+<0.01%)` | :arrow_up: |
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/18246?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/superset/pull/18246/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `96.25% <ΓΈ> (ΓΈ)` | |
| [...-frontend/src/explore/components/ControlHeader.jsx](https://codecov.io/gh/apache/superset/pull/18246/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sSGVhZGVyLmpzeA==) | | |
| [...-frontend/src/explore/components/ControlHeader.tsx](https://codecov.io/gh/apache/superset/pull/18246/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sSGVhZGVyLnRzeA==) | `82.60% <0.00%> (ΓΈ)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18246?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/18246?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 [c40b337...8c541e8](https://codecov.io/gh/apache/superset/pull/18246?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] villebro commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1029660984
Merging this for now, going to follow up later when we've had more time to iterate on the designs
--
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] rusackas commented on pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #18246:
URL: https://github.com/apache/superset/pull/18246#issuecomment-1027423912
@villebro / @kasiazjc I'm OK with it... I can't say it's love though. I appreciate the saved real estate, but it doesn't seem like the biggest real estate hog in its vicinity. As far as the interaction itself, If the menu/dropdown opens on _hover_, it's not so bad. But otherwise, we're asking for two clicks for a toggle, which seems cumbersome.
We could make the toggler smaller, and even use some AntD components, similar to [this](https://codepen.io/rusackas/pen/OJONzmq)
I think this PR solves the acute issue, so I'm approving... we can certainly continue to optimize if we feel like it, in this PR or another.
--
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] villebro merged pull request #18246: fix(listview): add nowrap to view mode container
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #18246:
URL: https://github.com/apache/superset/pull/18246
--
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