Skip to content

Conversation

@michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented Jan 13, 2021

SUMMARY

Fix list filters vertical alignment.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2021-01-13 at 11 36 42 AM

Screen Shot 2021-01-13 at 4 39 48 PM

TEST PLAN

1 - Go to main lists (databases, datasets, charts and dashboards)
2 - All filters labels should be vertically aligned with values

@junlincc

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Jan 13, 2021

Codecov Report

Merging #12497 (bd69d55) into master (7aba4c2) will decrease coverage by 7.11%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12497      +/-   ##
==========================================
- Coverage   66.29%   59.17%   -7.12%     
==========================================
  Files        1015      959      -56     
  Lines       49554    46832    -2722     
  Branches     5079     4344     -735     
==========================================
- Hits        32854    27715    -5139     
- Misses      16562    19117    +2555     
+ Partials      138        0     -138     
Flag Coverage Δ
cypress 50.94% <0.00%> (+3.83%) ⬆️
javascript ?
python 63.81% <81.81%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...t-frontend/src/components/ListView/Filters/Base.ts 100.00% <ø> (ø)
...end/src/views/CRUD/data/database/DatabaseModal.tsx 68.96% <0.00%> (+12.63%) ⬆️
superset/databases/schemas.py 99.45% <ø> (-0.55%) ⬇️
superset/db_engine_specs/druid.py 88.63% <33.33%> (+27.27%) ⬆️
superset/db_engine_specs/postgres.py 86.84% <89.47%> (+0.87%) ⬆️
...uperset-frontend/src/dashboard/util/dnd-reorder.js 0.00% <0.00%> (-100.00%) ⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js 0.00% <0.00%> (-100.00%) ⬇️
...dashboard/components/resizable/ResizableHandle.jsx 0.00% <0.00%> (-100.00%) ⬇️
.../src/dashboard/util/getFilterScopeFromNodesTree.js 0.00% <0.00%> (-93.48%) ⬇️
...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx 11.76% <0.00%> (-88.24%) ⬇️
... and 419 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40a334a...bd69d55. Read the comment docs.

@willbarrett
Copy link
Member

Hey @michael-s-molina this still looks a pixel low. Better than before though! Happy to merge as-is if there's an issue bumping that 1 more pixel up.

@michael-s-molina
Copy link
Member Author

michael-s-molina commented Jan 13, 2021

Hey @willbarrett you're absolutely right! Thanks for the eagle eyes! I'll fix it.

@rusackas
Copy link
Member

rusackas commented Jan 13, 2021

@michael-s-molina I see another route to fixing this that may make sense.

In Base.ts add align-items to FilterContainer, i.e.:

export const FilterContainer = styled.div`
  display: inline-flex;
  margin-right: 2em;
  font-size: ${({ theme }) => theme.typography.sizes.s}px;
  align-items: center;
`;

... and in FilterTitle remove the line-height, i.e.:

export const FilterTitle = styled.label`
  font-weight: bold;
  margin: 0 0.4em 0 0;
`;

then you should come out clean without having to nudge any pixels around.
image

@michael-s-molina michael-s-molina force-pushed the fix-list-filters-alignment branch from 4fc2346 to bd69d55 Compare January 13, 2021 19:37
@michael-s-molina
Copy link
Member Author

@rusackas Applied your suggestion. Thanks!

@rusackas
Copy link
Member

LGTM, but @nytai owns this so I'll let him 👀 & ✅ & 🔀

Copy link
Member

@nytai nytai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the fix!

@rusackas rusackas merged commit 87ac22b into apache:master Jan 13, 2021
@junlincc
Copy link
Member

let's get this one in for rc3. current state looks bad.

amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jan 14, 2021
@junlincc junlincc added the v1.0 label Jan 15, 2021
villebro pushed a commit that referenced this pull request Jan 15, 2021
villebro pushed a commit to preset-io/superset that referenced this pull request Jan 15, 2021
villebro pushed a commit to preset-io/superset that referenced this pull request Jan 15, 2021
etr2460 pushed a commit that referenced this pull request Jan 25, 2021
* release: bump to 1.0.0 and CHANGELOG

* fix(explore): long metric name display (#12387)

* fix(explore): long metric name display

* add tooltip to control

* chore: Show datasets when search input is empty (#12391)

* chore: Fix typo “Rest” to “Reset” (#12392)

* chore: upgrade eslint, babel, and prettier (#12393)

* feat(explore): add tooltip to timepicker label (#12401)

* chore: change Datasource to Dataset in Explore ui (#12402)

* chore(explore):change dataset to datasource in ui

* modal

* Add space

* Changing it back🤦🏾‍♀️

* Chargeback

* fix: Refresh Interval Modal dropdown (#12406)

* fix(native-filters): incorrect queriesData state (#12409)

* refactor: from superset.utils.core break down date_parser (#12408)

* Fixes control panel fields styling (#12236) (#12326)

* feat: Resizable dataset and controls panels on Explore view (#12411)

* Implement resizable panels on explore view

* Optimize chart rendering while resizing

* Make dataset column narrower

Co-authored-by: Evan Rusackas <evan@preset.io>

* fix(dashboard): artefacts shown while drag and dropping deck.gl charts (#12418)

* [12181] Fix artifacts while drag and dropping deck.gl charts.

* Run prettier

* bump superset-ui packages for rolling window change (#12426)

* chore: bump superset-ui deckgl plugin (#12466)

* fix: do not show vertical scrollbar for charts in dashboard (#12478)

* fix: do not show vertical scrollbar for charts in dashboard

* Proper fix for #11419

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>

* fix(dashboard): use datasource id from slice metadata (#12483)

* fix(timepicker): make pyparsing thread safe (#12489)

* fix: make pyparsing thread safe

* remove parenthesis for decorator

* fix (SQL Lab): disappearing results on tab switch (#12472)

* fix (SQL Lab): disappearing results on tab switch

* Remove state

* Fix test

* fix: import ZIP files that have been modified (#12425)

* fix: import ZIP files that have been modified

* Add unit test

* update changelog with rc2 entries

* fix: impose dataset ownership check on old API (#12491)

* fix: impose dataset ownership check on old API

* update UPDATING.md

* partially protect the old MVC also

* prevent metric and column add and update

* ci: remove refs/tags from docker tags on a release (#12518)

* ci: remove refs/tags from docker tags on a release

* wider head

* fix: lowercase all columns in examples (#12530)

* fix(explore): time table control panel (#12532)

* fix(explore): Add Time section back to FilterBox (#12537)

* Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YEARS (#12536)

* fix: Select options overflowing Save chart modal on Explore view (#12522)

* Fix select options overflowing modal

* fix unit test

Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>

* Fix list filters vertical alignment (#12497)

* feat(db-engine): Add support for Apache Solr (#12403)

* [db engine] Add support for Apache Solr

* Fixing typo

* chore: rename docker image in build_docker_image.sh, docker-compose.yml and helm values.yaml (#12337)

* add rc3 changelog entries

* fix: Popover closes on change of dropdowns values (#12410)

* fix: Add MAX_SQL_ROW value to LIMIT_DROPDOWN (#12555)

* fix(viz): missing groupby and broken adhoc metrics for boxplot (#12556)

* fix: height on grid results (#12558)

* fix: case expression should not have double quotes (#12562)

* Fix 500 error when loading dashboards with slice having deleted dataset (#12535)

* add rc4 changelog entries

* Fixed typo on line 348

* Added files

Co-authored-by: Daniel Gaspar <danielvazgaspar@gmail.com>
Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com>
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: Junlin Chen <junlin@preset.io>
Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
Co-authored-by: Agata Stawarz <47450693+agatapst@users.noreply.github.com>
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com>
Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com>
Co-authored-by: Grace Guo <grace.guo@airbnb.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Co-authored-by: Xiang Fu <fx19880617@gmail.com>
Co-authored-by: Ahmed Adel <github@aadel.io>
Co-authored-by: Amit Miran <47772523+amitmiran137@users.noreply.github.com>
Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>
Co-authored-by: Shuyao Bi <shuyaob@andrew.cmu.edu>
Co-authored-by: Lyndsi Kay Williams <lyndsikaywilliams@Lyndsis-MacBook-Pro.local>
@mistercrunch mistercrunch added 🍒 1.0.0 Cherry-picked to 1.0.0 🍒 1.0.1 Cherry-picked to 1.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS v1.0 🍒 1.0.0 Cherry-picked to 1.0.0 🍒 1.0.1 Cherry-picked to 1.0.1 🚢 1.2.0 First shipped in 1.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants