Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid resetting margin to 0 #6536

Merged
merged 2 commits into from
Dec 14, 2018

Conversation

michellethomas
Copy link
Contributor

Y axis labels were getting cut off at the top because margin.top was previously set to 30 and now gets reset to 0 without getting set again later. That top margin is necessary when a chart doesn't have a legend, the top gets cut off.

Fixes #6517

@mistercrunch @kristw

@codecov-io
Copy link

codecov-io commented Dec 14, 2018

Codecov Report

Merging #6536 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #6536   +/-   ##
======================================
  Coverage    73.5%   73.5%           
======================================
  Files          72      72           
  Lines       10002   10002           
======================================
  Hits         7352    7352           
  Misses       2650    2650

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 1fba6f7...3255f49. Read the comment docs.

@kristw kristw added !deprecated-label:bug Deprecated label - Use #bug instead .visualization labels Dec 14, 2018
@michellethomas
Copy link
Contributor Author

@mistercrunch I did a little more testing and noticed that the margin on top of the pie chart is back (something your pr was trying to get rid of), I added a line back to remove the margin just for pie, but lmk if there's a better way to handle this

@mistercrunch
Copy link
Member

The better way to handle this is to get rid of NVD3 :)
LGTM

@michellethomas michellethomas merged commit fe0f5e9 into apache:master Dec 14, 2018
@michellethomas michellethomas deleted the fix_chart_margin_top branch December 14, 2018 22:17
michellethomas added a commit that referenced this pull request Dec 18, 2018
* Avoid resetting margin to 0

* Fixing margin for pie chart

(cherry picked from commit fe0f5e9)
bipinsoniguavus pushed a commit to ThalesGroup/incubator-superset that referenced this pull request Dec 26, 2018
* Avoid resetting margin to 0

* Fixing margin for pie chart
conglei pushed a commit to conglei/incubator-superset that referenced this pull request Jan 7, 2019
* Avoid resetting margin to 0

* Fixing margin for pie chart
john-bodley pushed a commit that referenced this pull request Jan 11, 2019
* [SIP-3] Scheduled email reports for Slices / Dashboards (#5294)

* [scheduled reports] Add support for scheduled reports

* Scheduled email reports for slice and dashboard visualization
  (attachment or inline)
* Scheduled email reports for slice data (CSV attachment on inline table)
* Each schedule has a list of recipients (all of them can receive a single mail,
  or separate mails)
* All outgoing mails can have a mandatory bcc - for audit purposes.
* Each dashboard/slice can have multiple schedules.

In addition, this PR also makes a few minor improvements to the celery
infrastructure.
* Create a common celery app
* Added more celery annotations for the tasks
* Introduced celery beat
* Update docs about concurrency / pools

* [scheduled reports] - Debug mode for scheduled emails

* [scheduled reports] - Ability to send test mails

* [scheduled reports] - Test email functionality - minor improvements

* [scheduled reports] - Rebase with master. Minor fixes

* [scheduled reports] - Add warning messages

* [scheduled reports] - flake8

* [scheduled reports] - fix rebase

* [scheduled reports] - fix rebase

* [scheduled reports] - fix flake8

* [scheduled reports] Rebase in prep for merge

* Fixed alembic tree after rebase
* Updated requirements to latest version of packages (and tested)
* Removed py2 stuff

* [scheduled reports] - fix flake8

* [scheduled reports] - address review comments

* [scheduled reports] - rebase with master

* Typo: Fixed link (#6087)

* Update contributing.md with latest local dev instructions (#6513)

* Fix malformed table in docs/visualization.rst (#6409)

* Update requests version (#6510)

* Update requests version

* Run pip-compile --output-file requirements.txt

* [warm] Enforcing consistent form-data (#6531)

* Avoid resetting margin to 0 (#6536)

* Avoid resetting margin to 0

* Fixing margin for pie chart

* Refactor teradata to new time_grain_functions spec (#6539)

* Refactor teradata to new time_grain_functions spec

* Add test for time_grain_functions

* Add docker files to gitignore (#6507)

* add docker files to gitignore

* Update .gitignore

* adding in a dependency version to fix an error with Flask CLI (#6547)

* Fix string value displaying NaN (#6534)

* Documentation Correction to use http.server for Python3 (#6549)

* Remove note about snowflake-sqlalchemy stable version regression (#6398)

* filter_values documentation fix (#5977)

* Pass security manager to QUERY_LOGGER (#6548)

* Pass security manager to log_query

* Fix lint

* Increase size of column `name` in table `ab_view_meu` (#6532)

* Create migration script

* Use batch operation for ALTER COLUMN

* fix addr_str format bug (#6551)

* Fix deck.gl Polygon not show (#6545)

* Fix deck.gl Polygon not show

* Replace "//" to "// "

* Secure unsecured views and prevent regressions (#6553)

* Secure views and prevent regressions

* Force POST on shortner

* Fix tests

* [cosmetic] add css no-wrap on CRUD's last modified (#6522)

* [RfC] Fix URL too long (#6519)

When a URL gets too long (usually happens with controls that allow
free form text), it creates an array of bugs.
* bug when creating a short URL, because the POST request's referrer
field is too long
* a bug when saving a chart (when the modal shows up) also because of
the referrer being too long

Some work has been done in the past to mitigate this, and I'm unclear if
it's some sort of regression because of the Flask upgrade or some
browser change, or whether these bugs have always been around.

This is a request for comment on an approach that works. Let me know if
you can think of better ideas as to how to manage this.

My current solution looks for 8000+ characters URLs and shrinks them to
only `/superset/explore/?URL_IS_TOO_LONG_TO_SHARE&form_data=...` and we
only keep the formData keys for `datasource` and `viz_type`. Not super
elegant but does the trick.

* minor, auto tune debug mode when use flask_env (#6550)

* minor, auto tune debug mode when use flask_env

* update changes

* Make owner a m2m relation on datasources (#6544)

* Make owner a m2m relation on datasources

* Fix pylint

* Make migration work in mysql & sqlite

* Fix multiple db_migrations heads (#6560)

* [SIP-14] remove dependency on yarn in favor of npm (#6541)

* [SIP-14] remove dependency on yarn in favor of npm

Find more details here
#6217

* Add core-js to dev deps

* lint

* fix cypress

* break down cypress_build.sh

* fix cypress

* Use 'npm ci' in place of 'npm install'

* Bump npm req to >=6.5.0

* Enforcing  npm@'>=6.5.0'

* Using latest lts of nvm

* Make cache settings global

* remove uneeded 'npm run test' as 'npm run cover' takes care of that

* Prefix with 'time'

* Trying to upgrade babel-eslint instead of downgrading

* upgrading babel-polyfill to '@babel/polyfill'

* Moving to babel 7

* remove unused packages

* Make boto3/botocore installation optional (#6540)

* Make boto3 installation optional

* pylinting

* [SQL Lab] Allow running multiple statements (#6112)

* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

* Adding missing slash (#6567)

* [refactor] moving some datasource-related code to the frontend (#5769)

* [refactor] moving some datasource-related code to the frontend

* fix js tests

* fix tests

* fix test

* json_iso_dttm_ser use for TableViz (#6563)

* fix #6530 and add some other chinese translation (#6569)

* Fix: updated required cryptography version to 2.4.2 to resolve #6509 (#6516)

* Fix: updated required cryptography version to 2.4.2 to resolve #6509

* Fix: updated cryptography version to 2.4.2 to resolve #6509

* Fix: updated cryptography version to 2.4.2 to resolve #6509

* Revert "Fix: updated required cryptography version to 2.4.2 to resolve #6509"

This reverts commit b57b08c.

* Fix: updated cryptography version to 2.4.2

* Fix multi/dual when no right y axis (#6571)

* [design] use angle icons instead of carets for expandable panels (#6564)

* [design] use angle icons instead of carets for expandable panels

Also moving to the right to conform to the material design specs

* Fix cypress test

* fixing issue #6572 with Oracle date handling (#6580)

* fix Oracle engine specs for dates issue #6572

* fix Oracle engine specs for dates issue #6572

* fix Oracle engine specs for dates issue #6572, removing comment

* ng a trailing space

* Change margin for slice description in charts for the dashboard view (#6575)

Changed margin for the slices description in dashboard view, previously the margin was 5px for all the sides but for improve the view just apply margin for top and bottom.

* [bugfix] moving from reactable to reactable-arc fork (#6576)

* Using batch_op in db migration 0b1f1ab473c0 (#6581)

As I needed to downgrade from db migration 0b1f1ab473c0, I realized I
needed to use batch_op against SQLLite.

* Finish move to babel 7 (#6573)

* Finish move to babel 7

* Bump jest to 23.6.0

* Address comments

* Enhance Docker (#6504)

Refactored deprecated functionalities:
  - Used 'celery worker' command instead of 'superset worker' which is
    deprecated since 0.26.0
  - Used 'gunicorn' command instead of 'superset runserver'

* add chinese translate (#6592)

* Fix bug: some word not translate in js. (#6598)

delete the 'null,' in messages.json,
so that word can be translate in js.

* shift labels down along y-axis (#6596)

* Fix 'Uncaught TypeError: Cannot read property 'value' of undefined' #6556 (#6574)

closes #6556

* added get_data
;

* added cache logic

* added cache logic

* added cache key logic

* add missing part

* revised the metric part

* added to_dict

* fix lint

* fix lint

* fix lint

* fix lint

* fix lint

* fix flake8

* fix invalid name

* fix invalid name

* fix invalid name

* enable worldcloid

* working version

* rebase

* fixed expore link issue

* add endpoint

* added migration script WIP

* fixed lint

* remove migration script

* fix flake8

* fix tesT

* fix tesT

* fix tesT

* fix typo

* fixed metric format

* fixed metric format

* fixed metric format

* fixed tests

* fixed test

* fixed druid test

* fixed more tests

* fixed more tests

* fix test for viz

* fixed js tests

* fixed ts test

* fixed ts test

* fixed cypress test

* fixed the metric format in filterViz
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 27, 2024
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* Avoid resetting margin to 0

* Fixing margin for pie chart

(cherry picked from commit 7f0ff8c)
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 !deprecated-label:bug Deprecated label - Use #bug instead v0.30 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Y axis labels occasionally gets cut off at the top
4 participants