Skip to content
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

Add anchor to heading #26

Closed
wants to merge 1 commit into from

Conversation

minodisk
Copy link
Contributor

@minodisk minodisk commented Jul 2, 2015

Add anchor node to heading node for internal linking.

before:

<h2>
  <span>foo</span>
</h2>

after:

<h2>
  <a id="foo" class="anchor" href="#foo">
    <span class="icon icon-link"></span>
  </a>
  <span>foo</span>
</h2>

when 'tableHeader'
$ 'thead', {key}, [
$ 'tr', {key: key+'-_inner-tr'}, node.children.map (cell, i) ->
k = key+'-th'+i
$ 'th', {key: k, style: {textAlign: tableAlign[i] ? 'left'}}, toChildren(cell, k)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toChildrendefs 渡し忘れかと思って改変しています。

@mizchi
Copy link
Owner

mizchi commented Jul 9, 2015

@minodisk md2reactのようなモジュールを要求するようなウェブサイト、多くはSPA環境を想定しているのですが、最近のSPAはハッシュフラグメントをページ状態を表すのに使うことが多く、アンカーによるジャンプは邪魔になる可能性が高いです。

なので、デフォルトオフで無効にするオプションを提供しないと、このパッチを採用するのは難しいと思います。

@uasi uasi mentioned this pull request Jul 9, 2015
@uasi
Copy link
Collaborator

uasi commented Jul 9, 2015

@minodisk defs の修正は別途取り込ませていただきました 🙏

@minodisk
Copy link
Contributor Author

minodisk commented Jul 9, 2015

@mizchi レビューありがとうございます。確かにSPA前提だとすごく邪魔ですね。SPA側でルーティングから除外するような処理書くのもバカバカしいですし。
これ系の処理入れたい需要今後もありそうな気がしてて、その度にオプションを提供すると大変なことになりそうだなと思いました。

md2react は今のプレーンなエレメントを吐く。md2react を使う側からオプションでプラガブルに特定のノードタイプの処理をオーバーライトできるようにして、後は勝手にやってくれという設計ってどうでしょうか?

md2react raw,
  gfm: true
  overwrites:
    'heading': (node, defs, key) -> # ドメインに依った処理する
  • switch node.type
    switch node.type
    の先を node.type 毎に上書きできるようにする。
  • toChildren, getPropsFromHTMLNode, ATTR_WHITELIST など幾つかのユーティリティも export しつつ。

@uasi ありがとうございます。別の修正混ぜ込んじゃってごめんなさい 🙇

@mizchi
Copy link
Owner

mizchi commented Jul 9, 2015

@minodisk すべての子nodeがtoChildren通ってるので、ここでagnosticな関数を通すのはアリですね。
ただこの場合ids相当のことをするの難しいのでは?という気がしますが、どうでしょう

@minodisk
Copy link
Contributor Author

minodisk commented Jul 9, 2015

@mizchi

今回のケース、idstoChildrenに引き渡すのをやめちゃうとできそうな気がしています。

https://github.com/minodisk/markn/blob/ffaf4d45656353a7a634b65a990fff099354d82a/src/renderer/index.coffee#L35

createMdElement = (md) ->
  ids = {}
  md2react raw,
    gfm: true
    overwrites:
      'heading': (node, defs, key) ->
        text = node.children.filter (child) -> child.type is 'text'
          .map (child) -> child.value
          .join ''
        id = text.toLowerCase().replace(/\s/g, '-').replace(/[!<>#%@&='"`:;,\.\*\+\(\)\{\}\[\]\\\/\|\?\^\$]+/g, '')
        if !ids[id]?
          ids[id] = 0
        else
          ids[id]++
          id = "#{id}-#{ids[id]}"
        $ ('h'+node.depth.toString()), {key}, [
          $ 'a', {key: key+'-a', id: id, className: 'anchor', href: "##{id}"}, [
            $ 'span', {key: key+'-a-span', className: 'icon icon-link'}
          ]
          toChildren(node, defs, key, null)
        ]

@minodisk
Copy link
Contributor Author

設計の方針見えたと思うので閉じます。

@minodisk minodisk closed this Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants