Add inside / outside margins and todo outline #18

Tinggaard · 2024-11-25T11:58:28Z

As discussed in typst/packages#1315, a port of package to allow a note outline, and usage of inside / outside margins.

It does not take text orientation into account, as it is (to my knowledge) not inferrable. I was contemplating implementing it manually, but saw that it broke the logic for drawing the notes overall, so I disregarded it.

I'm not that experienced in writing in Typst yet, so please let me know if there could be improvements :)

ntjess · 2024-11-25T23:22:48Z

Thanks for the PR! I'm traveling for holidays this week but should be able to review the code afterward, if that timing is ok for you

Tinggaard · 2024-11-26T08:08:58Z

No worries

Tinggaard · 2024-12-10T15:54:05Z

Are there any updates @ntjess?

ntjess · 2024-12-12T05:43:08Z

Thanks for the reminder! It's been a particularly busy week for me, but I will get this reviewed tomorrow.

I appreciate your patience 🙂

Tinggaard · 2024-12-12T21:59:56Z

I was also considering adding additional styling to specify the size of the small upward line, as I'm fond of a smaller one in general. Like the ones shown here:

But I'm not 100% sure how the path drawing is achieved though, as it seemed to break the layout when I tried to change the size of the line at line 374

typst-drafting/drafting.typ

Lines 373 to 380 in 769fbbc

    
           let path-pts = _path-from-diffs( 
        
             (0pt, -1em), 
        
             (0pt, 1em + text-offset), 
        
             (-anchor-x + props.margin-left + 1 * pct, 0pt), 
        
             (-2 * pct, 0pt), 
        
             (0pt, dy), 
        
             (-1 * pct - box-width / 2, 0pt), 
        
           )

drafting.typ

typst.toml

drafting.typ

ntjess · 2024-12-13T02:44:19Z

I was also considering adding additional styling to specify the size of the small upward line

Something like this? Changing the property caret-height to a smaller value like 0.25em will match your image:

From 64d8c48616a8c74e1ca6e655a2fad1c99f91cd2c Mon Sep 17 00:00:00 2001
From: Nathan Jessurun <[email protected]>
Date: Thu, 12 Dec 2024 20:40:47 -0600
Subject: [PATCH] Allow configurable margin caret height

---
 drafting.typ | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drafting.typ b/drafting.typ
index a2ba157..96ca03a 100644
--- a/drafting.typ
+++ b/drafting.typ
@@ -33,6 +33,7 @@
     rect: rect,
     side: auto,
     hidden: false,
+    caret-height: 1em,
   ),
 )
 #let note-descent = state("note-descent", (:))
@@ -434,8 +435,8 @@
   let path-pts = _path-from-diffs(
     // make an upward line before coming back down to go all the way to
     // the top of the lettering
-    (0pt, -1em),
-    (0pt, 1em + text-offset),
+    (0pt, -props.caret-height),
+    (0pt, props.caret-height + text-offset),
     (dist-to-margin, 0pt),
     (0pt, dy),
     (1*pct + right-width / 2, 0pt)
@@ -464,8 +465,8 @@
   let text-offset = 0.4em
   let box-width = props.margin-left - 4 * pct
   let path-pts = _path-from-diffs(
-    (0pt, -1em),
-    (0pt, 1em + text-offset),
+    (0pt, -props.caret-height),
+    (0pt, props.caret-height + text-offset),
     (-anchor-x + props.margin-left + 1 * pct, 0pt),
     (-2 * pct, 0pt),
     (0pt, dy),
-- 
2.39.5 (Apple Git-154)

We can also have a margin-trail dict of props or something, if we want to customize more behaviors down the road

Tinggaard · 2024-12-13T17:57:28Z

Oh my, thanks for a comprehensive review!
~~I will get to it, but I also have plans for the weekend, so it may take a few days... Just FYI~~

Edit: This aged well...

Change page-binding to be `alignment` Set default note fill to `none` Move margin calculations to seperate function Determine text direction based on lang

Tinggaard · 2024-12-13T20:34:33Z

We can also have a margin-trail dict of props or something, if we want to customize more behaviors down the road

Not entirely sure what you mean by a margin trail... Is it the path from the location in the text, to the note you wish to customize?

Tinggaard · 2024-12-13T20:36:29Z

drafting.typ

@@ -398,7 +513,7 @@
 /// - dy (length): Vertical offset from the note's location -- negative values
 ///   move the note up, positive values move the note down
 /// - ..kwargs (dictionary): Additional properties to apply to the note. Accepted values are keys from `margin-note-defaults`.
-#let margin-note(body, dy: auto, ..kwargs) = {
+#let margin-note(body, dy: auto, ..kwargs) = [ #h(0pt) #{ // h(0pt) forces here().position() to take paragraph indent into account


I'm not really happy about this approach btw, if you have a suggestion...

But setting

#set par(first-line-indent: 1em)

Results in notes in a new paragraph not being place properly

ntjess · 2024-12-13T20:38:25Z

Indeed — in case users want e.g. smooth radius curves instead of rectangular paths or something. Just a thought if we are providing ways to customize the path

drafting.typ

Tinggaard · 2024-12-21T19:40:03Z

I've been using the @ntjess repo because it's handling right/left based on inside/outside margins, so I will provide a MRE showing the problem.

The package does currently not support inside/outside margins afaik, or am I missing something?

I see your problem @reverendpaco. The addition was made to allow for an outline entry such that all notes in the document can be listed, along with their content.
Maybe an option would be to make this feature opt-in, such that these figures are not created unless explicitly requested.

Tinggaard · 2024-12-21T19:46:13Z

I will see if I can make it work with labels instead of figures, as @ntjess suggested. This way the content would not be displayed twice.

reverendpaco · 2024-12-21T20:02:30Z

I've been using the @ntjess repo because it's handling right/left based on inside/outside margins, so I will provide a MRE showing the problem.

The package does currently not support inside/outside margins afaik, or am I missing something?

Huh.. that's funny. You're right. I have no idea how I got here then 😄 . It's especially funny because if I bothered to look at what your PR was, I could see that it has nothing to do with inside/outside auto-location.

reverendpaco · 2024-12-21T20:28:36Z

I've been using the @ntjess repo because it's handling right/left based on inside/outside margins, so I will provide a MRE showing the problem.

The package does currently not support inside/outside margins afaik, or am I missing something?

Huh.. that's funny. You're right. I have no idea how I got here then 😄 . It's especially funny because if I bothered to look at what your PR was, I could see that it has nothing to do with inside/outside auto-location.

Ok.. Sorry for confusing you and myself. I DO want to use your repo @Tinggaard . You are the one who introduces inside/outside margin (which I needed for my tufte-style layout). I was incorrect to say:

I've been using the @ntjess repo because it's handling right/left based on inside/outside margins

should have said:

I've been using the @Tinggaard repo because it's handling right/left based on inside/outside margins

I got your names/repos confused.

Tinggaard · 2024-12-23T09:35:42Z

@reverendpaco I have removed the hidden figure (#_note-outline-entry(props, body)), and queried using labels as @ntjess suggested.

I have also tested your provided MRE, and it does not create an additional footnote unless #note-outline() is invoked, creating an additional citation for the text in the outline.

Tinggaard · 2025-01-08T17:55:16Z

@ntjess, would you care to review again?

ntjess · 2025-01-13T17:51:28Z

Hey, sorry again for the delay. Yes, I'll be able to review this weekend if it's not too late? Otherwise, I can make time in the next two days.

I want to test a few edge cases which requires me to sit down and make the typst documents

Tinggaard · 2025-01-13T21:23:22Z

Take your time. I just wanted to make sure the PR was not forgotten :)

ntjess · 2025-01-26T20:08:49Z

tests/misc.typ

@Tinggaard any other good things to test here?

Hmm, that depends on the purpose of the test...
We could basically test all the functionality in margin-note-defaults, most important is probably something like hidden and caret-height.

Also, how is the test evaluated?

ntjess · 2025-01-26T20:32:16Z

drafting.typ

+  let props = (
+    fill: note.at("fill", default: defaults.fill),
+    stroke: note.at("stroke", default: defaults.stroke),
+    body: note.at("body", default: "<body text unkonwn>"),


Styled notes add things to the overview. I think it's fine for now, but we can think of a future PR to grab just the text

@Tinggaard Do you think it's desirable to copy the entire stroke instead of just the paint for the outline box, which will preserve dashing etc?

Yeah, we could consider using get.text() from t4t. This would remove all the styling at least.

I've had the same issue when using it locally and setting a smaller font-size in the notes, which makes the outline look weird if the styling is applied.

As for the stroke, I think it's nice to have the dash available, as well as possibly the thickness.

Tinggaard · 2025-01-27T17:14:09Z

drafting.typ

+    let paint = stroke(note-props.stroke).paint
+    link(
+      note.location().position(),
+      grid(
+        columns: (1em, 1fr, 10pt),
+        column-gutter: 5pt,
+        align: (top, bottom, bottom),
+        box(
+          fill: note-props.fill,
+          stroke: if note-props.stroke == none {
+            none
+          } else if paint != note-props.fill {


Maybe it is of lesser concern (as it seems weird to do), but this panics in the stroke constructor if note-props.stroke is none, which is possible (in the current version).

#set-margin-note-defaults(stroke: none)

Tinggaard added 3 commits November 25, 2024 11:31

Use inside / outside margins (closes ntjess#3)

4ea4263

Create outline of todos (closes ntjess#11)

6ba4d29

Update documentation with outline, fill and inside / outside margins

614a8de

Tinggaard force-pushed the main branch from 20d1e0d to 614a8de Compare November 25, 2024 12:00

Add all types of notes to outline

15eebe4

Tinggaard mentioned this pull request Nov 29, 2024

todonotes:0.1.0 typst/packages#1315

Open

8 tasks

Do not put outline on a seperate page

7a202d2

ntjess reviewed Dec 13, 2024

View reviewed changes

drafting.typ Outdated Show resolved Hide resolved

drafting.typ Outdated Show resolved Hide resolved

drafting.typ Outdated Show resolved Hide resolved

drafting.typ Outdated Show resolved Hide resolved

typst.toml Show resolved Hide resolved

drafting.typ Outdated Show resolved Hide resolved

Tinggaard added 2 commits December 13, 2024 21:07

Adjustments according to @ntjess's review

0ec6a43

Change page-binding to be `alignment` Set default note fill to `none` Move margin calculations to seperate function Determine text direction based on lang

Add caret-height to support smaller carets

9edfc3f

Tinggaard commented Dec 13, 2024

View reviewed changes

drafting.typ Outdated Show resolved Hide resolved

reverendpaco reviewed Dec 21, 2024

View reviewed changes

drafting.typ Outdated Show resolved Hide resolved

Query labels instead of figures in outline

b89a0d2

Tinggaard force-pushed the main branch from 293430a to b89a0d2 Compare December 23, 2024 09:31

ntjess added 4 commits January 26, 2025 13:58

More accurate default outline title

25e3a38

Format + update docs to tidy 0.4.1

d865f26

Add doc for regression testing

47278be

Better testing document

133dd7d

ntjess reviewed Jan 26, 2025

View reviewed changes

Handle custom/arbitrary note rects (like colorful-boxes)

f6919a7

ntjess reviewed Jan 26, 2025

View reviewed changes

Update readme

417ef1a

ntjess mentioned this pull request Jan 27, 2025

Towards a more flexible margin-note implementation #20

Open

Tinggaard commented Jan 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inside / outside margins and todo outline #18

Add inside / outside margins and todo outline #18

Tinggaard commented Nov 25, 2024

ntjess commented Nov 25, 2024 •

edited

Loading

Tinggaard commented Nov 26, 2024

Tinggaard commented Dec 10, 2024

ntjess commented Dec 12, 2024

Tinggaard commented Dec 12, 2024

ntjess commented Dec 13, 2024

Tinggaard commented Dec 13, 2024 •

edited

Loading

Tinggaard commented Dec 13, 2024

Tinggaard Dec 13, 2024

ntjess commented Dec 13, 2024

Tinggaard commented Dec 21, 2024

Tinggaard commented Dec 21, 2024

reverendpaco commented Dec 21, 2024

reverendpaco commented Dec 21, 2024

Tinggaard commented Dec 23, 2024

Tinggaard commented Jan 8, 2025

ntjess commented Jan 13, 2025

Tinggaard commented Jan 13, 2025

ntjess Jan 26, 2025

Tinggaard Jan 27, 2025

ntjess Jan 26, 2025

ntjess Jan 27, 2025

Tinggaard Jan 27, 2025

Tinggaard Jan 27, 2025

Add inside / outside margins and todo outline #18

Are you sure you want to change the base?

Add inside / outside margins and todo outline #18

Conversation

Tinggaard commented Nov 25, 2024

ntjess commented Nov 25, 2024 • edited Loading

Tinggaard commented Nov 26, 2024

Tinggaard commented Dec 10, 2024

ntjess commented Dec 12, 2024

Tinggaard commented Dec 12, 2024

ntjess commented Dec 13, 2024

Tinggaard commented Dec 13, 2024 • edited Loading

Tinggaard commented Dec 13, 2024

Tinggaard Dec 13, 2024

Choose a reason for hiding this comment

ntjess commented Dec 13, 2024

Tinggaard commented Dec 21, 2024

Tinggaard commented Dec 21, 2024

reverendpaco commented Dec 21, 2024

reverendpaco commented Dec 21, 2024

Tinggaard commented Dec 23, 2024

Tinggaard commented Jan 8, 2025

ntjess commented Jan 13, 2025

Tinggaard commented Jan 13, 2025

ntjess Jan 26, 2025

Choose a reason for hiding this comment

Tinggaard Jan 27, 2025

Choose a reason for hiding this comment

ntjess Jan 26, 2025

Choose a reason for hiding this comment

ntjess Jan 27, 2025

Choose a reason for hiding this comment

Tinggaard Jan 27, 2025

Choose a reason for hiding this comment

Tinggaard Jan 27, 2025

Choose a reason for hiding this comment

ntjess commented Nov 25, 2024 •

edited

Loading

Tinggaard commented Dec 13, 2024 •

edited

Loading