[IMP] zoom: Implement zoom for the grid #7232
Conversation
|
|
fdamhaut
force-pushed
the
master-zoom-flda
branch
2 times, most recently
from
0dc11ae to
a8282d4
Compare
fdamhaut
force-pushed
the
master-zoom-flda
branch
5 times, most recently
from
b173063 to
202f299
Compare
fdamhaut
force-pushed
the
master-zoom-flda
branch
4 times, most recently
from
810a2e6 to
2312dae
Compare
fdamhaut
force-pushed
the
master-zoom-flda
branch
from
2312dae to
1ecfee8
Compare
| options: { zoom?: number } = { zoom: 1 } | ||
| ) { | ||
| // const zoom = options.zoom || 1; |
There was a problem hiding this comment.
👀
and it doesn't seem to be used
| "getPositionAnchorOffset", | ||
| "getGridOffset", | ||
| "getViewportZoomLevel", | ||
| "getVisibleRectWithZoom", |
There was a problem hiding this comment.
nitpick: move this right next to "getVisibleRect",
| rect.x = rect.x * zoom + this.gridOffsetX + HEADER_WIDTH * (zoom - 1); | ||
| rect.y = rect.y * zoom + this.gridOffsetY + HEADER_HEIGHT * (zoom - 1); |
There was a problem hiding this comment.
we'll probably want to support zoom level for dashboards as well, especially in mobile
| icon: "o-spreadsheet-Icon.IRREGULARITY_MAP", | ||
| }; | ||
|
|
||
| export const zoomAction: (zoom: number) => ActionSpec = (zoom) => { |
There was a problem hiding this comment.
| export const zoomAction: (zoom: number) => ActionSpec = (zoom) => { | |
| export function zoomAction(zoom: number): ActionSpec { |
| top: Pixel; | ||
| left: Pixel; |
There was a problem hiding this comment.
if these are pixels, we usually name them x, y and we actually have a type for that: DOMCoordinates
It could be changed in a follow up commit
| this.env.model.dispatch("SET_ZOOM", { zoom: fontSize / 100 }); | ||
| } | ||
|
|
||
| onToggle() { |
There was a problem hiding this comment.
| onToggle() { | |
| toggle() { |
| get isActive() { | ||
| return this.topBarToolStore.isActive; |
There was a problem hiding this comment.
We don't really need this. Just use this.topBarToolStore.isActive the only place it's used
| class: string; | ||
| } | ||
|
|
||
| export class ToolBarZoom extends Component<Props, SpreadsheetChildEnv> { |
There was a problem hiding this comment.
There's probably room for factorization with TopBarFontSizeEditor if you want to give it a try
| min="min" | ||
| max="max" |
There was a problem hiding this comment.
| min="min" | |
| max="max" | |
| t-att-min="min" | |
| t-att-max="max" |
| const zv = ZOOM_VALUES; | ||
| describe.each(zv)("Edge-Scrolling on mouseMove of hightlights with zoom %s%", (zoomValue) => { |
There was a problem hiding this comment.
| const zv = ZOOM_VALUES; | |
| describe.each(zv)("Edge-Scrolling on mouseMove of hightlights with zoom %s%", (zoomValue) => { | |
| describe.each(ZOOM_VALUES)("Edge-Scrolling on mouseMove of hightlights with zoom %s%", (zoomValue) => { |
4 participants
Task: 4606648
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist