Skip to content

fix(readme): fix broken image links for proper rendering#499

Merged
Saurabh7019 merged 19 commits into
pnp:devfrom
DevPio:issue-473
Aug 22, 2025
Merged

fix(readme): fix broken image links for proper rendering#499
Saurabh7019 merged 19 commits into
pnp:devfrom
DevPio:issue-473

Conversation

@DevPio

@DevPio DevPio commented May 26, 2025

Copy link
Copy Markdown
Contributor

🎯 Aim

Fix broken image links in the README by updating relative paths to use raw.githubusercontent.com, ensuring proper rendering across environments like SharePoint.

📷 Result

image

✅ What was done

  • Updated README.md to replace broken relative image paths
  • Added logic to handle images/ path dynamically via script

🔗 Related issue

Closes: #473

@Saurabh7019

Copy link
Copy Markdown
Contributor

Thanks @DevPio, we will review this as soon as possible.

@Saurabh7019 Saurabh7019 self-assigned this May 27, 2025
@Saurabh7019

Copy link
Copy Markdown
Contributor

Hi @DevPio, nice to see the Chatbot Integration for SharePoint Online sample working now 👏 but, there are still some broken images in a few other samples:

  • Copilot April's Fool
  • Personal Anniversary
  • rEmployee Recognition
  • Mermaid Diagram Web Part for SharePoint Online

These are just a few examples, but there may be others with similar issues. Could you please check if the fix can address image loading across all samples?

@Saurabh7019 Saurabh7019 marked this pull request as draft May 27, 2025 19:27
@DevPio

DevPio commented May 27, 2025

Copy link
Copy Markdown
Contributor Author

Great, I was trying to find other examples of image breaking but I couldn't find any! I'll make sure it works for all cases. Thanks!

@DevPio

DevPio commented May 27, 2025

Copy link
Copy Markdown
Contributor Author

image
image
image
image
image
image
@Saurabh7019 I noticed that some of the examples you listed, except for rEmployee Recognition, the images are also broken in the repository. I believe the problem is in the readme of the repository itself. I accept suggestions in this case as to what we should do!

@Saurabh7019

Copy link
Copy Markdown
Contributor

a lot of issues are because of case-sensitive URLs (readme has Sample.png but the actual file is sample.png) and I don't know if there is an easy fix for this on our side without changing the markdown in the original repos, which we should avoid.
Unfortunately, when images appear broken in our sample gallery, it looks like the problem is with our implementation when it's not. I will create a separate issue for this and open for discussion.

For now, let's make sure that if the images load correctly in the sample repository README, they also appear correctly in the sample gallery detail view.

@DevPio DevPio marked this pull request as ready for review May 29, 2025 00:58
@Adam-it

Adam-it commented May 31, 2025

Copy link
Copy Markdown
Member

@DevPio @Saurabh7019
Hey guys I noticed this PR was closed by a PR in different PnP repo 🤔
Is this by design or should we reopen this one and merge it anyway?
Also I noticed a new package was introduced so I guess the npm-shrinkwrap.json should get updated right?
We need to double check this package if we are ok with introducing it. So like with any other package we should check if it is actively maintained and check for security issues.

@Saurabh7019

Copy link
Copy Markdown
Contributor

I am reopening this; I just saw Luccas' message on LinkedIn, the issue got closed due to an unintended cross repository reference.

@Saurabh7019 Saurabh7019 reopened this May 31, 2025
@Adam-it

Adam-it commented May 31, 2025

Copy link
Copy Markdown
Member

I am reopening this; I just saw Luccas' message on LinkedIn, the issue got closed due to an unintended cross repository reference.

Ok, thanks @Saurabh7019 for you awesome work in managing all this 👍
Please be sure to triple check the new dependency we are adding.
Also when testing be sure to build the vsix package and install the extensions from a file the same way as you would install it from a store. That's when the problems with es module show up which are not present when you run the extension from debug mode.

@Saurabh7019

Copy link
Copy Markdown
Contributor

@DevPio, Could you please rebase your branch to the latest dev to clean up the commit history and ensure only the relevant changes are included? also, could you please update the npm-shrinkwrap.json for the new dependency as Adam suggested above?

@Saurabh7019

Copy link
Copy Markdown
Contributor

Please be sure to triple check the new dependency we are adding.
Also when testing be sure to build the vsix package and install the extensions from a file the same way as you would install it from a store. That's when the problems with es module show up which are not present when you run the extension from debug mode.

Sure @adam, I will check it.

@Saurabh7019 Saurabh7019 marked this pull request as draft June 6, 2025 08:14
@Saurabh7019

Copy link
Copy Markdown
Contributor

Could you please rebase your branch

@DevPio, when you get a chance, could you please rebase your branch to dev? please let me know if you need any help with this.

@DevPio DevPio marked this pull request as ready for review June 7, 2025 16:58
@Saurabh7019 Saurabh7019 marked this pull request as draft June 7, 2025 20:12

@Adam-it Adam-it left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@DevPio I checked your dependencies after rebase and only noticed one thing we need to fixup manually

Comment thread npm-shrinkwrap.json Outdated
@DevPio DevPio marked this pull request as ready for review June 9, 2025 21:49
@Saurabh7019

Copy link
Copy Markdown
Contributor

Hi @DevPio, thanks for rebasing, we are almost there.. just a couple more conflicts left in npm-shrinkwrap.json and package.json. When you get a chance, could you please rebase with the latest dev again and resolve those? Please let me know if you need anything from me.

@Saurabh7019 Saurabh7019 marked this pull request as draft June 12, 2025 07:19
@Saurabh7019

Copy link
Copy Markdown
Contributor

Hi @DevPio, hope you are well. Just checking in on this PR.. Please let us know if there's anything we can help with.

@DevPio

DevPio commented Jul 23, 2025

Copy link
Copy Markdown
Contributor Author

Hi @Saurabh7019 , I believe all that's left is to finalize the merge, the implementation is ready, I'll need your help to validate, all the issues you pointed out have been fixed. I'll set aside some time to close this merge

@Saurabh7019

Copy link
Copy Markdown
Contributor

Hi @DevPio, there are still a couple of merge conflicts that need resolving before we can finalize. Could you please rebase with latest dev and mark the PR ready for review when done?

@Adam-it

Adam-it commented Aug 13, 2025

Copy link
Copy Markdown
Member

hey, we just had a new minor release, which means our dev branch was rebased with main.
Please align your feature branch with dev before moving forward with this change 👍

@DevPio

DevPio commented Aug 18, 2025

Copy link
Copy Markdown
Contributor Author

Finally resolved the conflicts 🎉 @Saurabh7019 Could you please check if everything looks good? I’m available for any further adjustments and sorry for the delay 🙏

@DevPio DevPio marked this pull request as ready for review August 18, 2025 00:51

@Saurabh7019 Saurabh7019 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Impressive work, @DevPio. Thanks for this change. All samples are loading fine now.
There are only a few minor issues. Let's fix those before we merge.

Comment thread npm-shrinkwrap.json Outdated
Comment thread npm-shrinkwrap.json
Comment thread src/webview/view/components/gallery/DetailsView.tsx Outdated
Comment thread src/webview/view/components/gallery/DetailsView.tsx Outdated
Comment thread src/webview/view/components/gallery/DetailsView.tsx Outdated
Comment thread src/webview/view/components/gallery/DetailsView.tsx Outdated
@DevPio DevPio requested a review from Saurabh7019 August 22, 2025 03:19
@Saurabh7019 Saurabh7019 merged commit 7b2a52a into pnp:dev Aug 22, 2025
1 check passed
@Saurabh7019

Copy link
Copy Markdown
Contributor

Merged, Thanks for the fix!

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.

3 participants