fix: control panel fields styling (#12236) #12326

michael-s-molina · 2021-01-07T11:42:17Z

SUMMARY

Adjusted METRICS and FILTER to use 4px border radius
Adjusted GROUP BY to keep same margin as METRICS and FILTERS
Adjusted RATIO height, border and box shadow
Adjusted METRICS, FILTERS and GROUP BY border colors
Adjusted search height and box shadow
Adjusted pills border color
Adjusted Add annotation layer button size
Changed Select component to present placeholder bellow the box when multi = true

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Closes #12236

@junlincc

TEST PLAN

1 - Select a chart
2 - View changes in control panel
3 - Repeat this process for charts with different control panel fields

ADDITIONAL INFORMATION

codecov-io · 2021-01-07T12:28:15Z

Codecov Report

Merging #12326 (58bc312) into master (4cf3f99) will decrease coverage by 4.37%.
The diff coverage is 78.26%.

@@            Coverage Diff             @@
##           master   #12326      +/-   ##
==========================================
- Coverage   66.79%   62.41%   -4.38%     
==========================================
  Files        1014     1014              
  Lines       49524    49512      -12     
  Branches     5077     5078       +1     
==========================================
- Hits        33079    30905    -2174     
- Misses      16314    18397    +2083     
- Partials      131      210      +79

Flag	Coverage Δ
cypress	`?`
javascript	`60.71% <78.26%> (+<0.01%)`	⬆️
python	`63.53% <ø> (-0.54%)`	⬇️

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

Impacted Files	Coverage Δ
...end/src/components/Select/SupersetStyledSelect.tsx	`88.29% <ø> (-2.13%)`	⬇️
...nd/src/explore/components/ExploreViewContainer.jsx	`2.54% <0.00%> (-77.71%)`	⬇️
...nd/src/explore/components/controls/TextControl.tsx	`42.85% <ø> (-26.20%)`	⬇️
superset-frontend/src/components/Select/styles.tsx	`61.33% <70.00%> (-24.78%)`	⬇️
superset-frontend/src/components/Label/index.tsx	`100.00% <100.00%> (ø)`
...rontend/src/explore/components/DatasourcePanel.tsx	`85.71% <100.00%> (-4.99%)`	⬇️
...frontend/src/explore/components/OptionControls.tsx	`42.85% <100.00%> (-32.76%)`	⬇️
.../src/explore/components/controls/SelectControl.jsx	`86.91% <100.00%> (-6.42%)`	⬇️
superset-frontend/src/SqlLab/App.jsx	`0.00% <0.00%> (-100.00%)`	⬇️
superset-frontend/src/explore/App.jsx	`0.00% <0.00%> (-100.00%)`	⬇️
... and 203 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 4cf3f99...58bc312. Read the comment docs.

adam-stasiak · 2021-01-07T12:54:49Z

My scenario:
Built everything with command:
CYPRESS_CONFIG=true docker-compose up
Then opened chart:

Then expanded advanced analytics

I observed Rolling Function glitch:

I compared this to version from different branch:

adam-stasiak · 2021-01-07T13:28:21Z

Could you verify as well situation when we try to click on

And then try to expand dropdowns:
dropdown.mov

@adam-stasiak I just confirmed that this is happening in master. Can you open a new issue and remove this comment so we can keep only related issues?

Yes sure :)

zhaoyongjie · 2021-01-07T14:13:06Z

same style problem at the latest master branch. so we need new PR fix thit issue.

michael-s-molina · 2021-01-07T14:16:40Z

same style problem at the latest master branch. so we need new PR fix this issue.

@zhaoyongjie No problem. I'll fix this issue in this PR.

junlincc · 2021-01-07T14:18:28Z

thank you @adam-stasiak for spotting the issue!
if the code looks good and we can confirm the styling issue preexists. let's merge this PR.
@michael-s-molina please open up a separate PR for the rolling function glitch,. thanks so much fo the fix!

michael-s-molina · 2021-01-07T14:21:26Z

thank you @adam-stasiak for spotting the issue!
if the code looks good and we can confirm the styling issue preexists. let's merge this PR.

We can't merge it just yet. Although this problem exists in master, some inputs became bigger and when we hover them we have some layout problems. I'm fixing this right now and I'll also fix the preexisting problem.

zhaoyongjie · 2021-01-07T14:21:33Z

LGTM, thank you for your fix.

@michael-s-molina @junlincc

superset-frontend/src/explore/components/OptionControls.tsx

michael-s-molina · 2021-01-07T17:29:10Z

@nikolagigic Accepted all your suggestions. Thanks.

I increased the width (320px to 380px) of both panels to support bigger inputs. We still have the situation of layout flicks when we hover elements because there is no space available for the tooltip icon and this creates a line break. In talks with @junlincc she told me that this is being fixed in another PR where we're going to stack all controls and this line break problem won't happen anymore. So I will limit the scope of this PR to just fixing input styles.

For documentation purposes these images bellow show some examples where a line break happens:

junlincc · 2021-01-07T17:42:40Z

scope of the issue/pr should be limited to styling changes. which are done!
@pkdotson and @mihir174 will open a separate PR to address the layout issue. 🙏 thanks all!

rusackas · 2021-01-07T20:18:19Z

superset-frontend/stylesheets/less/variables.less

This is going to affect (apparently) at least 68 other things in the codebase. Is this a situation where we could use Emotion's styled to apply this color in a more specific place (using theme.colors.grayscale.light2)?

Actually that was the intention because #e0e0e0 is the official gray-light theme color.

rusackas · 2021-01-07T20:19:27Z

superset-frontend/stylesheets/less/cosmo/variables.less

Since this is part of the broader bootstrap theme, I'm a little worried this might cause some other regression somewhere else in the application. Have you done some kind of visual audit, or could we make this change in the relevant component(s) with Emotion styling?

I did a visual audit of most screens and I didn't found any problem. It shouldn't cause any problem because this is a configurable variable of Bootstrap. To limit the scope of this change we need to track all components that use classes that depend on this variable and apply Emotion styles individually.

junlincc

move placeholder below the box so it does not take up to whole row and enlarge the box

adjust the color of the pill border as well

pls adjust the annotation box size as well

michael-s-molina · 2021-01-08T19:24:37Z

@junlincc I made all the requested changes.

junlincc

this PR makes my day! thanks for all the px and color perfect styling and moving the "placeholder" options out of the box!

The only issue left is the rolling window styling that should have fixed in Superset-UI. let's make sure @rusackas

rusackas

There are a couple of lines in the global (bootstrap) styles that make me a little nervous, but I haven't seen any fallout yet :)

rusackas · 2021-01-11T20:10:45Z

Hoping E2E tests go through on this new run, will merge if so!

junlincc · 2021-01-11T20:25:46Z

@mihir174 want to bring this to your awareness

junlincc · 2021-01-11T20:55:08Z

@pkdotson need to confirm the layout issues in advanced analytics are being taken care of 🙏

pkdotson · 2021-01-11T21:04:28Z

rolling window will be fixed in this upcoming pr: apache-superset/superset-ui#895

junlincc · 2021-01-12T00:39:34Z

while we tested again, we found blank page of deck.glMultiple Layers. and confirm that this has been an issue, not related to this PR.

This PR is good to go if codes look ok!

* 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>

superset-github-bot bot added the Turing label Jan 7, 2021

pull-request-size bot added the size/S label Jan 7, 2021

michael-s-molina changed the title ~~Fixes control panel fields styling (#12236)~~ fix: control panel fields styling (#12236) Jan 7, 2021

zhaoyongjie self-requested a review January 7, 2021 14:20

zhaoyongjie approved these changes Jan 7, 2021

View reviewed changes

nikolagigic reviewed Jan 7, 2021

View reviewed changes

superset-frontend/src/explore/components/OptionControls.tsx Outdated Show resolved Hide resolved

nikolagigic reviewed Jan 7, 2021

View reviewed changes

superset-frontend/src/explore/components/OptionControls.tsx Outdated Show resolved Hide resolved

junlincc added the explore:control Related to the controls panel of Explore label Jan 7, 2021

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from 8f0aa0b to 6d29c02 Compare January 7, 2021 17:16

nikolagigic approved these changes Jan 7, 2021

View reviewed changes

junlincc mentioned this pull request Jan 7, 2021

[Explore]Advanced Analytics section rolling window layout #12333

Closed

junlincc self-requested a review January 7, 2021 18:52

rusackas reviewed Jan 7, 2021

View reviewed changes

junlincc reviewed Jan 8, 2021

View reviewed changes

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from 6d29c02 to 9b513fa Compare January 8, 2021 19:03

pull-request-size bot added size/M and removed size/S labels Jan 8, 2021

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from 9b513fa to 0714b73 Compare January 8, 2021 19:22

junlincc mentioned this pull request Jan 9, 2021

[Explore]control fields line break on hover #12374

Closed

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from 0714b73 to ef180ce Compare January 9, 2021 17:58

pull-request-size bot added size/L and removed size/M labels Jan 9, 2021

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from ef180ce to 43d372c Compare January 11, 2021 14:36

Fixes control panel fields styling (apache#12236)

58bc312

michael-s-molina force-pushed the fix-control-panel-fields-styling branch from 43d372c to 58bc312 Compare January 11, 2021 14:42

junlincc approved these changes Jan 11, 2021

View reviewed changes

junlincc requested a review from mistercrunch January 11, 2021 16:47

rusackas approved these changes Jan 11, 2021

View reviewed changes

junlincc added the rush! Requires immediate attention label Jan 11, 2021

junlincc added the hold:testing! On hold for testing label Jan 11, 2021

junlincc mentioned this pull request Jan 12, 2021

[Explore]Blank deck.gl Multiple Layers chart control(Old issue) #12427

Closed

rusackas merged commit 75f927b into apache:master Jan 12, 2021

junlincc removed the hold:testing! On hold for testing label Jan 12, 2021

villebro added the v1.0 label Jan 13, 2021

villebro pushed a commit that referenced this pull request Jan 13, 2021

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

507302d

amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jan 14, 2021

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

bc9d713

junlincc removed the rush! Requires immediate attention label Mar 18, 2021

fix: control panel fields styling (#12236) #12326

fix: control panel fields styling (#12236) #12326

Uh oh!

Conversation

michael-s-molina commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Uh oh!

codecov-io commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

adam-stasiak commented Jan 7, 2021

Uh oh!

adam-stasiak commented Jan 7, 2021

Uh oh!

zhaoyongjie commented Jan 7, 2021

Uh oh!

michael-s-molina commented Jan 7, 2021 • edited by zhaoyongjie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junlincc commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-s-molina commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhaoyongjie commented Jan 7, 2021

Uh oh!

Uh oh!

Uh oh!

michael-s-molina commented Jan 7, 2021

Uh oh!

junlincc commented Jan 7, 2021

Uh oh!

rusackas Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

rusackas Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

michael-s-molina Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

junlincc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michael-s-molina commented Jan 8, 2021

Uh oh!

junlincc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

rusackas commented Jan 11, 2021

Uh oh!

junlincc commented Jan 11, 2021

Uh oh!

junlincc commented Jan 11, 2021

Uh oh!

pkdotson commented Jan 11, 2021

Uh oh!

junlincc commented Jan 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

michael-s-molina commented Jan 7, 2021 •

edited

Loading

codecov-io commented Jan 7, 2021 •

edited

Loading

michael-s-molina commented Jan 7, 2021 •

edited by zhaoyongjie

Loading

junlincc commented Jan 7, 2021 •

edited

Loading

michael-s-molina commented Jan 7, 2021 •

edited

Loading

junlincc left a comment •

edited

Loading

junlincc left a comment •

edited

Loading