Alternate \page{curlies}

[calculuschild]

Description

Takes #3899 and moves the handling of curlies back into Markdown.js. Small adjustments to regex, etc.

Opening as a new PR as I was getting too many merge conflicts to easily track. Note that these changes only apply to V3. Legacy should be unaffected.

Usage:

Similar to an inline single mustache.

\page{wide,banna=fail,color:black}

Related Issues or Discussions

Solves #3901

Reviewers, refer to this list when testing features, or suggest new items

• Verify new features are functional
  • Adding a style includes a style on the <div class='page'> instance.
  • Adding a class includes a class on the <div class='page'> instance.
  • Adding an HTML Attribute adds the attribute on the <div class='page'> instance.
  • Adding an ID does not add it to the <div class='page'> instance.
  • Mixed forms of above work as expected
  • CSS styles with hyphens (e.g. line-height) work (Page is a React component, which requires camelCase input)
  • Curlies mix properly with other pages styles (e.g. the page "shadow" option from the toolbar)
  • \page on the first line of a document will not create a page break, and is counted as page 1
• Verify old features have not broken
  • \page works without values.
  • Highlighting of \page still works
  • Page number display is accurate
  • Jumping to source/brew still works in every case (including Legacy)
• Test for edge cases / try to break things
  • Ensure CSS Properties ( --FOO ) can be set.
• Identify opportunities for simplification and refactoring
• Check for code legibility and appropriate comments

Copy the list here

- [ ] Verify new features are functional
  - [ ] Adding a style includes a style on the `<div class='page'>` instance.
  - [ ] Adding a class includes a class on the `<div class='page'>` instance.
  - [ ] Adding an HTML Attribute adds the attribute on the `<div class='page'>` instance.
  - [ ] Adding an ID does ***not*** add it to the `<div class='page'>` instance.
  - [ ] Mixed forms of above work as expected
  - [ ] CSS styles with hyphens (e.g. `line-height`) work (Page is a React component, which requires camelCase input)
  - [ ] Curlies mix properly with other pages styles (e.g. the page "shadow" option from the toolbar)
  - [ ] `\page` on the first line of a document will *not* create a page break, and is counted as page 1
- [ ] Verify old features have not broken
  - [ ] `\page` works without values.
  - [ ] Highlighting of `\page` still works
  - [ ] Page number display is accurate
  - [ ] Jumping to source/brew still works in every case (including Legacy)
- [ ] Test for edge cases / try to break things
  - [ ] Ensure CSS Properties ( --FOO ) can be set.
- [ ] Identify opportunities for simplification and refactoring
- [ ] Check for code legibility and appropriate comments

[5e-Cleric]

Using an empty inject breaks the syntax, which doesn't make much sense, allow for \page{}

[calculuschild]

Using an empty inject breaks the syntax, which doesn't make much sense, allow for \page{}

Fixed.

[calculuschild]

Any final comments, @5e-Cleric , @dbolack-ab , @ericscheid ?

[5e-Cleric]

Right now, when setting style for the first page, spaces are being allowed between page break and style inject, but not for other pages. Additionally, these spaces, while they don't break the regex for the inject, do break the syntax highlight.

[5e-Cleric]

Also, class or id injection doesn't work for either ( \page{#myPageId,myPageClass}

[5e-Cleric]

Will update these as i go

• Verify new features are functional
  • Adding a style includes a style on the <div class='page'> instance.
  • Adding a class includes a class on the <div class='page'> instance.
  • Adding an HTML Attribute adds the attribute on the <div class='page'> instance.
  • Adding an ID does not add it to the <div class='page'> instance.
  • Mixed forms of above work as expected
  • CSS styles with hyphens (e.g. line-height) work (Page is a React component, which requires camelCase input)
  • Curlies mix properly with other pages styles (e.g. the page "shadow" option from the toolbar)
  • \page on the first line of a document will not create a page break, and is counted as page 1
• Verify old features have not broken
  • \page works without values.
  • Highlighting of \page still works
  • Page number display is accurate
  • Jumping to source/brew still works in every case (including Legacy)
• Test for edge cases / try to break things
  • Ensure CSS Properties ( --FOO ) can be set.
• Identify opportunities for simplification and refactoring
• Check for code legibility and appropriate comments

[5e-Cleric]

/page in the first line in legacy creates a page, not in v3, i assume this is intended

[5e-Cleric]

Custom properties --foo are not being set, furthermore, a custom property with the same name as a norrmal property is added as that normal property, so --border is added as border.

[calculuschild]

Right now, when setting style for the first page, spaces are being allowed between page break and style inject, but not for other pages. Additionally, these spaces, while they don't break the regex for the inject, do break the syntax highlight.

I see what is going on here... First page is the only one that might go into the Marked parser without being filtered through the page split regex first which wasn't allowing spaces. Ok. So to align with the other injector behavior, I should allow spaces in every case and fix the syntax highlighting to allow spaces too.

Also, class or id injection doesn't work for either ( \page{#myPageId,myPageClass}

Good catch; somehow I deleted the class part when doing merges. Will put that back in. ID's are intentionally left out, else it would mess with page selection on the toolbar, etc.

/page in the first line in legacy creates a page, not in v3, i assume this is intended

Correct. This PR only applies to V3.

Custom properties --foo are not being set, furthermore, a custom property with the same name as a norrmal property is added as that normal property, so --border is added as border.

I see what happened. When I added the camelCase conversion it also converts --my-custom-variable to myCustomVariable. I will tweak that line to exclude converting properties starting with --.

[calculuschild]

@5e-Cleric I have fixed all of these cases you reported. Can you confirm?

[5e-Cleric]

I don't think it is a problem, but the order of attributes is different, can we fix that?

Otherwise i say this is a go. Do we want test coverage in this PR or later? Should be fairly easy.

[calculuschild]

Can you clarify what steps result in the different order?

As for testing, this just uses the same curly injection functions the rest of our markdown uses. The difference being we have to manually place the properties into a React component instead of just emitting the HTML output. So the parsing of the curlies is already tested.

As far as testing that pages get the right properties, I'm not fully sure how we would go about testing HTML output from React. I am thinking we put it off for later unless you know of a good strategy.

[5e-Cleric]

Can you clarify what steps result in the different order?

Making the same inject in page 1 vs page 2, with some text in the middle, not sure either.

[calculuschild]

Making the same inject in page 1 vs page 2, with some text in the middle, not sure either.

Interesting. I'm not sure how much control we have over the order React chooses here. Given that Page 1 is the only different one, my best guess is it has something to do with the very first "empty" render before any pages have been split and parsed, But this doesn't impact the behavior of the document at all so I don't know if it's worth hunting down.

I think I will merge this and we can decide if its worth the effort to clean up the ordering later.