set default charts layout #5425
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
graceguo-supercat
force-pushed
the
gg-DefaultChartsLayout
branch
2 times, most recently
from
3a2f7c4 to
a7831a1
Compare
williaster
reviewed
Jul 18, 2018
| const rowContainer = newComponentFactory(ROW_TYPE); | ||
| layout[rowContainer.id] = rowContainer; | ||
| parent.children.push(rowContainer.id); | ||
| if (newSlicesCounter % 3 === 0) { |
There was a problem hiding this comment.
I think a more robust way to do this is to use the grid width from constants.js and the actual width of the newly-created chartHolder component below. If the current row doesn't have enough width for the new component, then you can add a new row.
Your approach would break if someone unknowingly changed the default chart width.
There was a problem hiding this comment.
fixed according to comment.
graceguo-supercat
force-pushed
the
gg-DefaultChartsLayout
branch
2 times, most recently
from
a7c9ebe to
414e883
Compare
graceguo-supercat
force-pushed
the
gg-DefaultChartsLayout
branch
from
414e883 to
d4328ea
Compare
Codecov Report
@@ Coverage Diff @@
## master #5425 +/- ##
==========================================
- Coverage 63.37% 63.36% -0.02%
==========================================
Files 349 349
Lines 22110 22117 +7
Branches 2455 2457 +2
==========================================
+ Hits 14013 14014 +1
- Misses 8083 8089 +6
Partials 14 14
Continue to review full report at Codecov.
|
graceguo-supercat
pushed a commit
to graceguo-supercat/superset
that referenced
this pull request
Aug 14, 2018
(cherry picked from commit daf2116)
wenchma
pushed a commit
to wenchma/incubator-superset
that referenced
this pull request
Nov 16, 2018
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Users might be able to add charts into dashboard programmatically, without setting layout for those new charts. So I added a function that group those newly added charts 3 item in a row, attached the bottom of dashboard. This is the same behavior as in the old dashboard.
also some of our users feel default chart size is too small (3 out of 12 columns grid). So i make it as big as old dashboard (4 out of 12 columns grid).
@williaster