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

Implement method to process attributes that can be overridden by slots #2511

Closed luminousleek closed 2 months ago

luminousleek commented 2 months ago

What is the purpose of this pull request?

Partially resolves #2476

Overview of changes:

Implement processSlotAttribute method and use it for attributes that can be overridden by slots.

Anything you'd like to highlight/discuss:

For reference, these are the attributes that can be overridden by slots: Component Attributes
Panel Header, Alt
Box Icon, Header
Popover Header, Content
Modal Header
Dropdown Header
Scroll-top-button Icon
Question Header, Hint, Answer
Q-Option Reason
Quiz Intro
Tooltip Content
Tab Header
A-Point Header, Content, Label

The docs are not correct regarding this.

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)

Implement processSlotAttribute method

There is some duplicated code to process attributes that can
be overridden by slots. In addition, not every attribute that can be
overridden with slots are processed with the 
hasSlotOverridingAttribute method. As such, there will be no
warning in the logger when these attributes are overridden.

Let's put this duplicated code into a method and call it for every
attribute that can be overridden by a slot.

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).
luminousleek commented 2 months ago

The check_docs test is failing because the PR is doing its job - it's giving logger warnings when attributes for a quiz are being overridden: Screenshot 2024-04-10 at 4 21 56 PM

The warnings arise from the panel in the Header and Hint example with slots at this page (the relevant documentation file is /docs/userGuide/syntax/questions.md), where the code for the question in the panel is

<question type="checkbox" header="Which of the following is true?" hint="Think out of the box! :fas-box:">
  <!-- Header slot -->
  <div slot="header">
<p>Which of the following is correct?</p>
<p>Challenge: Try to get all the answers on your first try! ⭐️ ⭐️</p>
  </div>
<!-- other code omitted for brevity -->
  <!-- Hint slot -->
  <div slot="hint">
<p>Think out of the box! <span aria-hidden="true" class="fas fa-box"></span></p>
<p>Need another hint? <tooltip content="Two of the answers are correct!">Hover over me!</tooltip> <span aria-hidden="true" class="fas fa-mouse-pointer"></span></p>
  </div>
</question>

As can be seen, the header and hint attributes are overridden by their respective slots, which then triggers a logger warning. A simple fix would be to delete the header and hint attributes, so I'm wondering if it's OK to do this in this PR or if I need to create a separate PR for this.

luminousleek commented 2 months ago

I tested the rest of the other attributes for the components to see whether they're able to be overridden by slots.

Looks like the header, content and label attributes for a-point are able to be overridden by slots Screenshot 2024-04-12 at 1 02 29 PM

Also, the header attribute for tab is also able to be overridden by a slot Screenshot 2024-04-12 at 1 26 55 PM

As well as the content attribute for tooltip Screenshot 2024-04-12 at 1 34 01 PM

And the alt attribute for panel Screenshot 2024-04-12 at 1 39 29 PM

Basically that's all the attributes that are handled by MdAttributeRenderer.ts. The PR is updated to use processSlotAttributes for the rest of the attributes.

github-actions[bot] commented 2 months ago

@kaixin-hc Each PR must have a SEMVER impact label, please remember to label the PR properly.