-
-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: disable attributes copy to parent #157
Conversation
|
||
section.title > h1:first-child { | ||
} | ||
|
||
.level1 { | ||
} | ||
.level2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
属性コピーを無効化したので関連する記述を削除しました。
src/plugins/section.ts
Outdated
@@ -80,7 +42,12 @@ const sectionize = (node: any, ancestors: Parent[]) => { | |||
endIndex > 0 ? endIndex : undefined, | |||
); | |||
|
|||
const hProperties = checkProperties(node, depth); | |||
const hProperties = { class: [`level${depth}`] }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
階層の深さ class は残します。
// {hidden} specifier | ||
if (Object.keys(node.data.hProperties).includes('hidden')) { | ||
node.data.hProperties.hidden = 'hidden'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkProperties
の hidden 属性処理を移植しました。{}
記法で {hidden}
としてもこの属性は出力されないようです。この処理はその対策と思われるので checkProperties
削除後も互換のために維持しておきます。
}); | ||
const expected = `<section class="level1"><h1 id="foo">Heading <em>test</em></h1></section>`; | ||
expect(received).toBe(expected); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
仕様変更によるテスト修正です。
あわせて utils の利用を廃止しました。utils は整形された MDAST と HTML の両方を判定するのですが以下の理由により利用をやめてゆきます。
- HTML をテストすれば十分
- expect がテスト対象と別ファイルにあると失敗時の出力がわかりにくい
src/plugins/section.ts
Outdated
const hProperties = checkProperties(node, depth); | ||
const hProperties = { | ||
class: [`level${depth}`], | ||
'aria-labelledby': `heading-${depth}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
要望通り、新規に aria-labelledby 属性を追加しました。値は深さに連動します。
hidden属性などHTMLのブール属性は そこで hidden 以外のブール属性として autofocus 属性でテストしてみました。結果は、 # Foo {autofocus}
↓
<h1 id="foo">Foo</h1> # Foo {autofocus=""}
↓
<h1 id="foo">Foo</h1> # Foo {autofocus=autofocus}
↓
<h1 autofocus id="foo">Foo</h1> 興味深いことに、HTML仕様でブール属性として定義されていない属性や未定義の属性については空の値でもその属性が出力されます。 # Foo {tabindex aaa bbb="" title="title"}
↓
<h1 tabindex="" aaa="" bbb="" title="title" id="foo">Foo</h1> このテスト結果から、hidden 属性について特別に処理する必要があった理由は分かりました。hidden 属性以外についても同様に処理するとよいのでないかと思います。つまり、 それから、見出し以外での [Foo](#foo){hidden}
↓
<p><a href="#foo">Foo</a></p> |
aria-labelledby 属性はラベルとなる要素の id を指定するものです。sectionにとってはその子要素である見出し要素がラベルになるので見出しの id をsectionのaria-labelledby 属性に設定します。 このコメントに書いたHTMLの例、 <section class="level1" aria-labelledby="heading-1">
<h1 class="one" id="heading-1">Heading 1</h1>
<p>One.</p>
<section class="level2" aria-labelledby="heading-2">
<h2 class="two" id="heading-2">Heading 2</h2>
<p>Two.</p>
</section>
</section> この例での |
このPRのタイトルは "feat: disable attributes copy to parent" なので、見出し(→section)だけでなく画像(→figure)についての変更も、このPRに入れるとよいと思います。
Pandocでの変換の例:
|
ひとつの PR として対応する機能が多すぎるので分割したほうがよさそうです。
とします。平日は時間が取れなさそうなので来週末以降になります。 |
…ttribute In addition, the type definitions of KeyValue are commonized.
@MurakamiShinyu レビューをお願いします。 |
動作確認しました。よいと思います。 ドキュメントの次のところ:
見出しには id が自動生成されるので常に id 属性があるのでないでしょうか? そうであれば、 |
@MurakamiShinyu よって、単に見出しの |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
This resolves the problem on VFM v2 change: - vivliostyle/vfm#157 With the following Markdown, ```md # Table of Contents {.toc} - [One](one.html) - [Two](two.html) ``` VFM v1 generates the following HTML: ```html <section class="level1 toc" id="table-of-contents"> <h1 class="toc">Table of Contents</h1> <ul> <li><a href="one.html">One</a></li> <li><a href="two.html">Two</a></li> </ul> </section> ``` Vivliostyle.js recognizes the section as TOC because "toc" class is found in the section element. This is changed in VFM v2. It generates the following HTML: ```html <section class="level1" aria-labelledby="table-of-contents"> <h1 class="toc" id="table-of-contents">Table of Contents</h1> <ul> <li><a href="one.html">One</a></li> <li><a href="two.html">Two</a></li> </ul> </section> ``` The "toc" class is not found in the section element, so the TOC detection has to be tweaked using the selector, `section:has(>:first-child:is(h1,h2,h3,h4,h5,h6):is(.toc,#toc))`. Note: the stylesheet for TOC that was a part of UserAgentBaseCSS is now a separate stylesheet UserAgentTocCSS. This change is necessary because uaStylesheetBaseFetcher cannot handle the `:has()` selector.
refs #151
# Heading {hidden}
が処理されない{}
記法の仕様かバグ対策と思われるので、互換性のため踏襲heading-N
を設定するようにした