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

fix bug where column settings were dropped #11154

Merged
merged 1 commit into from
Oct 21, 2019
Merged

Conversation

paulrosenzweig
Copy link
Contributor

Resolves #11113

I think this broke in #10864. I had forgotten a ... 🤦‍♂. This PR adds the ... and a test.

@mazameli
Copy link
Contributor

@kdoh and/or @tlrobinson, mind giving this a review?

@@ -145,7 +145,9 @@ export function applyChartTimeseriesXAxis(
const timestampFixed = moment(timestamp)
.utcOffset(dataOffset)
.format();
const { column, columnSettings } = chart.settings.column(dimensionColumn);
const { column, ...columnSettings } = chart.settings.column(
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting, I though spreading undefined (which columnSettings would have been) would have crashed, but apparently you can spread null/undefined. I guess I can stop doing things like { ...(something || {}) }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah! I just had the same realization a few days ago!

@paulrosenzweig paulrosenzweig merged commit 7a64501 into release-0.33.x Oct 21, 2019
@paulrosenzweig paulrosenzweig deleted the fix-11113 branch October 21, 2019 19:50
mazameli pushed a commit that referenced this pull request Oct 21, 2019
* hide section picker if when viewing column settings

* hide sidebar title

* add ChartSettingsSidebar test

* always show column settings title

* lint

* override sidebar title

* remove unneeded diff

* group reference sidebar tables by schema

* keep everything as one list

* use name instead of display_name

* add tests

* add schema pane

* filter to querable tables

* update tests

* unused imports

* sort schema and table names

* change scalar compact formatting to depend on pixel width rather than grid width (#10932)

* prompt for save when sharing unsaved question (#10976)

* use generic props override instead of just `title` and `onBack`

* Update pulse table style to match app (#10989)

* undefined -> null, you can spread null aparently

* Upgrade redshift driver to 1.2.36.1060 (#11181)

* fix bug where column settings were dropped (#11154)

* remove leading slash on publicPath (#11174)

* Fix GA crashes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants