Add vertical ruler for writer #9030
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
from
409ade8
to
29eaab2
Compare
|
CORE Patch: https://gerrit.libreoffice.org/c/core/+/167481 |
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
2 times, most recently
from
1eac0ff
to
7a37843
Compare
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
4 times, most recently
from
5d37294
to
402f5ac
Compare
gokaysatir
requested changes
May 14, 2024
There was a problem hiding this comment.
Okay, for vertical ruler: We only have header and footer margins that are draggable as far as i can see.
So i guess we shouldn't have code for break points inside VRuler file?
And also it seems to me that vertical ruler goes off screen as i play with the header footer settings :) It feels like something related to left / right margins are forgotten. This is a guess. Can you check that vertical ruler doesn't go off screen and is readable.
I'd check the code from scratch and make some polishing (if required).
Thanks @Darshan-upadhyay1110 :) We are so close.
Yes we do not need break point for vertical ruler
Can you please give me hint, how did you change the header footer settings ? What action you performed that made ruler goes out of screen ? |
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
2 times, most recently
from
9a03648
to
c132659
Compare
Of course, i used page settings and set the header & footer margins. |
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
from
c99a8cd
to
f70d8be
Compare
Darshan-upadhyay1110
changed the title
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
3 times, most recently
from
27ceaa5
to
ffcb1e6
Compare
eszkadev
reviewed
May 22, 2024
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
3 times, most recently
from
662baf6
to
3507764
Compare
eszkadev
reviewed
May 23, 2024
eszkadev
reviewed
May 23, 2024
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
from
3507764
to
6980a8c
Compare
- add new LOK callback for vertical & horizontal ruler update Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I3c0e26f22072de4612e128d58ac41b629be82807 Add vertical ruler and implement margin calculation - Added new vertical ruler file to separate it from horizontal ruler calculation - For vertical ruler we do not need tab stops so removed it. - next stpe will be to implement add top and bottom margin by drag Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I8186c874ff59571db7a75ec4c31f81fc655ef301
- Before this commit vertical ruler stays only in first page
- if we jum to page 2,3 so and so ruler won't change it's possition
- but i have added some calculation based data we recevice from CORE
- calculation:
- we need to consider pageoffset for vertical ruler
- check if cursor moves to other page
- then add total Height of previous pages.
- for Ex: if we are at page 3 then ruler.marginInlineStart should additionally have total height ( page1Height + page2Height)
- this will place ruler to exact possition where we want
- next : add Top and Bottom margin drag
Signed-off-by: Darshan-upadhyay1110 <[email protected]>
Change-Id: Ie2a2d4e5f0cfd627bcafd81230ad33b7aec280a2
Signed-off-by: Darshan-upadhyay1110 <[email protected]>
- As we do for horizontal ruler we need marker that will change margin of Header and Footer of the document - current patch will have design where user can see markers in vertical ruler - you can drag and change potition of marker. Improvments that still need to do: - user can drag but it will not change size/margin of header/Footer - next we will pass info to CORE according to corrdinates we got from drag - it will update text potition on document Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I9663a75d5c84eace784266c76be12abd762434bb Signed-off-by: Darshan-upadhyay1110 <[email protected]>
- Send Upeer and Lower space value for header and footer according to drag - we need to send both values in core. - even if we drag either upper or lower we will send values accordingly but both values should be updated Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I4ef3287c5afe6e1eef5aac225d3c06ca9d1fd9e5
- we are string previous response values of top/bottom margin - resone is because we need that values to pass after dragEnd. - but if we change section foucs from Header to Footer or Text area to Shape then we need to reset that old values with the incoming response - this patch will do that. - for `table` we will hide the marker as we do not need that. table already have there marker to resize. - added some edge cases like if start marker goes beyond end marker - start and end marker should not cross limit beyond page top/bottom Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: Iffd3cbf08806666b858d9c3da7ca6362085223dd Signed-off-by: Darshan-upadhyay1110 <[email protected]>
- remove use of Leaflet's - change working js code to ts - please check above commit messages for understanding of implementation. Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: Iee222bbdb343730014260c1785a90e68c6dedfd1
Darshan-upadhyay1110
force-pushed
the
private/darshan/add-vertical-ruler-in-writer
branch
from
6980a8c
to
6cf5bdf
Compare
eszkadev
approved these changes
May 23, 2024
|
One UX comment:
I would expect to have both of them draggable and working. |
|
@gokaysatir you requested changes, please re-check |
gokaysatir
approved these changes
May 24, 2024
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.
Change-Id: I3c0e26f22072de4612e128d58ac41b629be82807
Summary
TODO
Checklist
make checkmake runand manually verified that everything looks okay