MarkBind / markbind

MarkBind is a tool for generating content-heavy websites from source files in Markdown format
https://markbind.org/
MIT License
134 stars 123 forks source link

Support text-icons of lists #2520

Closed yiwen101 closed 2 months ago

yiwen101 commented 2 months ago

What is the purpose of this pull request?

Overview of changes: Extends IconAttributeDetail with texts: TextsManager that set the text attributes of incoming nodes in run time according to rule. Very straightforward and least intrusive.

Anything you'd like to highlight/discuss: The input format remain under discussion.

Testing instructions: Can test with this:

1. Text inheritance test with text-icons of lists
* Item A { texts="['11','12','13','14']" }
* Item B
  * Sub-item B1 { texts="['21','22','23','24']" }
  * Sub-item B2
  * Sub-item B3
    * Sub-sub-item B3.1 { texts="['31','32','33','34']" }
    * Sub-sub-item B3.2
      * Sub-sub-sub-item B3.2.1
  * Sub-item B4

Proposed commit message: (wrap lines at 72 characters) Support text-icons of lists


Checklist: :ballot_box_with_check:


Reviewer checklist:

Indicate the SEMVER impact of the PR:

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).
damithc commented 2 months ago

@yiwen101 can users use an escape sequence (e.g., \,) if they want the , to be part of the text label?

yiwen101 commented 2 months ago

@yiwen101 can users use an escape sequence (e.g., \,) if they want the , to be part of the text label?

Hi prof, this is not supported in current implementation of this PR. But it should be easy to add.

In our discussion beneath, Yu Chen also suggests the alternative of using <div id="demo" data-stuff='["some", "string", "here"]'></div> as the syntax to parse an array of string. In that way, the "," can be used in the icon. I feel the syntax is a big clumsy as I do not like typing quotes repeatedly, but if we support both syntax, it can also serve as a good walk-around to the "," not supported issue without compromising ease of use.

damithc commented 2 months ago

but if we support both syntax, it can also serve as a good walk-around to the "," not supported issue without compromising ease of use.

Yup, that seems like a good solution.

damithc commented 2 months ago
  • I think this feature wouldn't be hard to extend for icons as well. Would this be needed @damithc ?

@yucheng11122017 for now, I don't see myself using this feature for icons, as icons usually don't occur in a natural sequence. So, let's not add that feature yet.

yiwen101 commented 2 months ago

Thanks for the work @yiwen101! Some code quality stuff.

Some thoughts

  • I think this feature wouldn't be hard to extend for icons as well. Would this be needed @damithc ?
  • What should the expected behavior be if a subsequent bullet point uses {text="OVERRIDE"}?

Something like this

* Item A { texts="11,12,13,14" }
* Item B
  * Sub-item B1 { texts="21,22,23,24" }
  * Sub-item B2 { text="OVERRIDE" }
  * Sub-item B3
    * Sub-sub-item B3.1 { texts="31,32,33,34" }
    * Sub-sub-item B3.2
      * Sub-sub-sub-item B3.2.1
  * Sub-item B4

would produce this: image The text is overriden which I think should be the correct behavior - users should be able to override with individual text. But 22 onwards will continue ie we dont skip the it. Should this be the desired behaviour?

Thank you for pointing out this, this undesirable behavior is indeed my neglect. I have corrected it in the latest PR. I have also update the documentation and test accordingly.

I have also updated the supported input format to "['a','b']" to solve the , cannot be used issue.

yiwen101 commented 2 months ago

LGTM besides some small nits

Thank you for the valuable inputs, insightful discussion on input format, thoughtful suggestion on the regex pattern and detailed examination for the documentation!!!

damithc commented 2 months ago

Feature in action: https://nus-cs2103-ay2324s2.github.io/website/admin/ip-w5.html

image