jackyzha0 / quartz

🌱 a fast, batteries-included static-site generator that transforms Markdown content into fully functional websites
https://quartz.jzhao.xyz
MIT License
7.14k stars 2.51k forks source link

Remove links for pages that don't exist #454

Open quantumgardener opened 1 year ago

quantumgardener commented 1 year ago

I believe it is bad design to link to pages that we know do not exist. It is ok to do with within Obsidian, but not on a public facing site. It goes against end user's expectations of clicking on a link and having it go where it suggests.

If I have [[Missing Link]] on a page, and have not provided "Missing Link.md" then when parsing the wikilinks, I would prefer to see no link, rather than a link which goes nowhere.

Simply put, iIf the target page of a wiki link from Obsidian is not in the list of known pages, do not link.

Some people may like this behaviour, so there could be options in the config, 1) as currently works 2) do not create link 3) link, but with a css tag indicating the link is missing (Obsidian Publish) does this.

I've looked at the code but don't have the ability to work out exactly what change is required.

thelulzy commented 11 months ago

If I have [[Missing Link]] on a page, and have not provided "Missing Link.md" then when parsing the wikilinks, I would prefer to see no link, rather than a link which goes nowhere.

Just want to confirm that not everyone would like this behavior!

Like you said, if such functionality is added it should be configurable!

FabianUntermoser commented 11 months ago

I agree this would be a really nice feature and much needed when you use a single vault for your public and private notes.

In addition to it being configurable, I think it would also be nice to be able to style those links differently, vs. removing the link in its entirety.

auctumnus commented 3 weeks ago

Hi! I made a quick solution for this but it's definitely not good enough for inclusion into actual quartz (I'm almost certain this would be slow for a large number of files):

// quartz/plugins/emitters/contentPage.tsx, line 109
const allSlugs = allFiles.map((f) => f.slug ? resolveRelative(slug, f.slug) : "")

visit(tree as Root, "element", (elem) => {
  if (elem.tagName === "a" && elem.properties.href) {
    const href = elem.properties.href.toString()
    if (!allSlugs.includes(href as RelativeURL)) {
      if(elem.properties.className === undefined) {
        elem.properties.className = "dead-link"
      } else if(Array.isArray(elem.properties.className)) {
        if(elem.properties.className.includes('external')) {
          return
        }
        elem.properties.className.push("dead-link")
      } else if (typeof elem.properties.className === "string") {
        if(elem.properties.className.includes('external')) {
          return
        }
        elem.properties.className += " dead-link"
      }
    }
  }
})

(You will also need to update imports.) This will add a .dead-link class to all dead links, by 1. making a list of all slugs, relative to the current file, and 2. traversing the entire AST looking for links, testing if they have an href that leads to any of the slugs in that list, and adding the .dead-link class if not.

I've only tested this on some example files, but it seems to work correctly so far.

quantumgardener commented 3 weeks ago

I have had a fix for this running a long time. My approach was to hit the ofm.ts transformer. Lines 241+ in my code. I'm only checking page to page links. Links to assets will still get through.

      // Only link pages that actually exist in the graph
      if (plainSlugs.includes(fp.replaceAll(" ","-").replaceAll("&","-and-").toLowerCase())) {
        return `${embedDisplay}[[${fp}${displayAnchor}${displayAlias}]]`
      } else {            
        const strippedAlias = displayAlias.split("|")[1]
        if (anchor) {
          return `${fp}<span class="anchor-split"/>${strippedAlias}`
        } else {
          return strippedAlias ?? fp
        }
      }
aarnphm commented 3 weeks ago

right now the link would just show as 404 page?

saberzero1 commented 3 weeks ago

Disabling links is not recommended, especially for accessibility reasons. I suggest replacing the broken links with a span element instead. (probably with styling similar to a normal link, just dimmer)

Another problem I see with checking links in the transformer step, is that pages might be filtered out in the filter step. Ultimately the removal or replacing of dead links should probably happen in the emitting step.

quantumgardener commented 3 weeks ago

There are a couple of issues here which will help explain my approach.

My workflow also relies on this. I have public pages that link to private pages (not in the /content folder). Perfect for my notes in Obsidian and doesn't create noise for people online. To activate the link, I move the page to /content.

For accessibility, I'm not disabling a link. I'm preventing it's creation. I do not understand how creating a link to a page that does not exist, however it is displayed, assists accessibility. There is nothing to access.

I'm doing the check in the transformer only because that's where I found was the best place to do so. It if can work elsewhere, then I'm ok with that.

A good solution would be to give option to keep current behaviour, show link in a different manner (i.e. span as suggested), or do not create the link.

saberzero1 commented 3 weeks ago

For accessibility, I'm not disabling a link. I'm preventing it's creation. I do not understand how creating a link to a page that does not exist, however it is displayed, assists accessibility. There is nothing to access.

My point was about NOT creating a link. So instead of <a href="" disabled>dead link</a>, use a non-anchor element instead, or none at all. Your screenreader users will be pleased.

My workflow also relies on this. I have public pages that link to private pages (not in the /content folder). Perfect for my notes in Obsidian and doesn't create noise for people online. To activate the link, I move the page to /content.

I'm doing the check in the transformer only because that's where I found was the best place to do so. It if can work elsewhere, then I'm ok with that.

This only works if you do not filter out any pages, which based on your workflow description seems to fine, but not everyone shares your workflow. Using your suggested implementation without using your workflow might not filter out all dead links.

auctumnus commented 3 weeks ago

Disabling links is not recommended, especially for accessibility reasons. I suggest replacing the broken links with a span element instead. (probably with styling similar to a normal link, just dimmer)

Oops! Yeah, that's true. This is what I ended up changing it to:

visit(tree as Root, "element", (elem) => {
  if (elem.tagName === "a" && elem.properties.href) {
    const href = elem.properties.href.toString()

    if(href.startsWith('#')) {
      return
    }

    if (!allSlugs.includes(href as RelativeURL)) {
      if (elem.properties.className === undefined) {
        elem.properties.className = "dead-link"
      } else if (Array.isArray(elem.properties.className)) {
        if (elem.properties.className.includes("external")) {
          return
        }
        elem.properties.className.push("dead-link")
      } else if (typeof elem.properties.className === "string") {
        if (elem.properties.className.includes("external")) {
          return
        }
        elem.properties.className += " dead-link"
      } else {
        return
      }
      elem.tagName = "span"
    }
  }
})

and yeah, I filter a lot of pages (with explicitPublish), so doing this in the transform step (like quantumgardener's approach) is non-workable.