hicommonwealth / commonwealth

A platform for decentralized communities
https://commonwealth.im
GNU General Public License v3.0
67 stars 44 forks source link

Do IPFS links need to be supported in the new Markdown viewer? #9240

Open burtonator opened 1 month ago

burtonator commented 1 month ago

Describe the bug

Right now we support this within the quill renderer:

markdownRenderer.image = (href, title, text) => {
  if (href?.startsWith('ipfs://')) {
    const hash = href.split('ipfs://')[1];
    if (hash) {
      href = `https://ipfs.io/ipfs/${hash}`;
    }
  }
  return `<img alt="${text}" src="${href}"/>`;
};

This fixes all img URLs so they load from https://ipfs.io not the ipfs:// scheme.

Is this used? I'll have to patch the img plugin in MDXEditor/Lexical to support this. I think it shouldn't be too hard to implement though.

@dillchen @jnaviask @timolegros

Initial conditions

Environment:

Branch/Release version:

Browser:

Wallet:

Reproduction steps

Actual behavior

Expected behavior

Screenshots / Video

Reporter

Additional context

timolegros commented 1 month ago

Is this code ever used? I tried to render an image from IPFS in the Quill editor and I can't get anything to render. I'm not sure how this would work in the first place since we can't tell when content the IPFS hash is pointing to unless we fetched it i.e. the hash could point to a PDF document instead of an image.

In general, I think it should be up to the user to ensure that the IPFS links they use are maintained. The user may provide an IPFS link to content they have pinned on their node. IMO it is not our responsibility to pin IPFS content the user has uploaded outside of Common or to ensure that that content is continually available. Per https://github.com/hicommonwealth/commonwealth/issues/9216, any and all images that we render in the editor should be from S3. Loading content from IPFS can be slow and links can break even if we use ipfs.io.

An example image hash to test with: QmSgvgwxZGaBLqkGyWemEDqikCqU52XxsYLKtdy3vGZ8uq -> https://ipfs.io/ipfs/QmSgvgwxZGaBLqkGyWemEDqikCqU52XxsYLKtdy3vGZ8uq

burtonator commented 1 month ago

@timolegros yeah. I agree with this. Though technically someone can link to an external image manually if/when we enable direct markdown mode. They could just paste/type the image URL directly into a markdown image reference.

Right now it's not possible though.

I'd like to NOT enable this too as it will be a big hairy and require me to have another patch for the img plugin for MDXEditor and that might not be accepted.

dillchen commented 1 month ago

All good, don't need

Sent via Superhuman @.***>

On Tue, Sep 17, 2024 at 5:09 PM, Kevin Burton @.***> wrote:

@timolegros https://github.com/timolegros yeah. I agree with this. Though technically someone can link to an external image manually if/when we enable direct markdown mode. They could just paste/type the image URL directly into a markdown image reference.

Right now it's not possible though.

I'd like to NOT enable this too as it will be a big hairy and require me to have another patch for the img plugin for MDXEditor and that might not be accepted.

— Reply to this email directly, view it on GitHub https://github.com/hicommonwealth/commonwealth/issues/9240#issuecomment-2356939928, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIWMHNPGAHQYNG3YKJAR4LZXCK7TAVCNFSM6AAAAABOJ56XN6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJWHEZTSOJSHA . You are receiving this because you were mentioned.Message ID: @.***>