[FIX] renderer: Fix Box rendering #5400
Open
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
|
|
rrahir
force-pushed
the
16.0-fix-non-aligned-merge-text-rar
branch
9 times, most recently
from
f89ead2 to
f40408e
Compare
hokolomopo
reviewed
Jan 6, 2025
| @@ -222,7 +222,7 @@ export class InternalViewport { | |||
| * @param zone | |||
There was a problem hiding this comment.
Typos in commit msg "dimensio", "taht"
Comment on lines
+120
to
+121
| "context.rect(0, 0, 952, 974)", | ||
| "context.clip()", |
There was a problem hiding this comment.
I would probably do a test that we clip what we dran on the frozen panes. It shouldn't be too annoying
There was a problem hiding this comment.
I added some tests related to the renderer and he new getter 'getRect'. Not sure about the naming but 'getRenderingRect' was definitely wrong. it's more used as a computational tool.
rrahir
force-pushed
the
16.0-fix-non-aligned-merge-text-rar
branch
from
f40408e to
963a33d
Compare
When we implemented the frozen panes, we accidently destroyed the distinction between the actual size of a cell/merge and its visual dimension in the viewport. i.e. the dimension of the part that we be rendered. However, the rest of the rendering process was relying on the box size to compute several things (button , text positions for instance) This means that all computations that relied on the actual size of a cell/merge are now false. A good example is the merge. Create a merge with some text inside of it and make sure it spans over several columns. The text is aligned at the center of the merge. Now scroll a bit and the text will stay centered in the VISIBLE part of the merge but not the merge itself. On another hand, we did not account for some text formatting, specifically text aligned on the right can overflow outside of their pane. This revision fixes the two issues: - Reintroduce a getter `getRect` to properly compute the positiontioning of text/icons of a box - Change the rendering process of the grid by splitting the rendering one pane at a time. Task: 4448426
rrahir
force-pushed
the
16.0-fix-non-aligned-merge-text-rar
branch
from
963a33d to
1695ab3
Compare
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.
When we implemented the frozen panes, we accidently destroyed the
distinction between the actual size of a cell/merge and its visual
dimensio in the viewport. i.e. the dimension of the part that we be
rendered.
However, the rest of the rendering process was relying on the box size
to compute several things (button , text positions for instance)
This means that all computations taht relied on the actual size of a
cell/merge are now false.
A good example is the merge. Create a merge with some text inside of it
and make sure it spans over several columns.
The text is aligned at the center of the merge. Now scroll a bit and the
text will stay centered in the VISIBLE part of the merge but not the
merge itself.
On another hand, we did not account for some text formatting, specifically
text aligned on the right can overflow outside of their pane.
This revision fixes the two issues:
getRectto properly compute thepositiontioning of text/icons of a box
one pane at a time and work in reverse such that a frozen pane takes
precedence on the main viewport.
The second part of the approach is not the panacea as the mobile mode
will push the concept of frozen pane further but as a fix it suits well.
Task: 4448426
review checklist