100pah commented on code in PR #20865:
URL: https://github.com/apache/echarts/pull/20865#discussion_r2085047661
##########
src/coord/cartesian/GridModel.ts:
##########
@@ -52,17 +53,17 @@ class GridModel extends ComponentModel<GridOption>
implements CoordinateSystemHo
show: false,
// zlevel: 0,
z: 0,
- left: '10%',
- top: 60,
- right: '10%',
- bottom: 70,
+ left: tokens.size.xl,
+ top: 65,
+ right: tokens.size.xl,
+ bottom: 60,
// If grid size contain label
- containLabel: false,
+ containLabel: true,
Review Comment:
That is a huge breaking change.
I think it should not be `true` by default, because we have set it `false`
for years and a large number of charts are layout based on it. Make that be
breaking brings less benefit but probably bother existing users by breaking the
existing layout.
And when there are multiple charts in one page (quite common in Management
Information System), axes between different charts are usually needed to be
aligned, which requires `containLabel: false`
And after set it back to `false`, we should go back to the original left
right setting `'10%'`
Regarding what's the ideal setting of `containLabel`:
- `containLabel: false` may cause long labels out of viewport
- `containLabel: true` loose the ability to align axis lines between
different charts.
I'm trying to implement a feature that reconcile them, that is,
`left/right/top/bottom/width/height` indicate a rect for axis lines (in
`containLabel: false` way), but if the labels are out of viewport, shrink the
rect to avoid it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
