Skip to content

Fix broken images in VPN page - #150

Merged
alexpeterson91 merged 1 commit into
dappnode:masterfrom
Hugoo:master
Apr 5, 2022
Merged

alexpeterson91 merged 1 commit into
dappnode:masterfrom
Hugoo:master

Conversation

@Hugoo

@Hugoo Hugoo commented Apr 4, 2022 •

Copy link
Copy Markdown
Contributor

EDIT: created issue: #151

Before PR:

image

After PR:

image

If images are in the static folder, you can simply use /img/........ instead of ../../../img -> this will make it more bulletproof if you change pages folders

@alexpeterson91 alexpeterson91 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.

this error you show isn't showing for me so i don't see a reason to change the paths right now when really nothing seems to be wrong.
EDIT.
Please create or find an open issue before submitting a PR for any of our repos so we have everything fully documented, with all context. It can be done here https://github.com/dappnode/DAppNodeDocs/issues.
I'll go ahead and create an issue and link it. But there are just a few other images that aren't rendering likely for the same reason throughout the docs, so while I can likely merge this shortly; the issue i create about improperly rendered images will remain open. We would absolutely love if you want to help fix or at least look into any of those other docs image issues.

@Hugoo

Hugoo commented Apr 5, 2022 •

Copy link
Copy Markdown
Contributor Author

this error you show isn't showing for me so i dont see a reason to change the paths right now when nothing really seems to be wrong.

Ahah i love it - hello? - super welcoming repo here 😳

"I don't see the error" 😅😅😅

Are you making software just for you?

Do you really think I spent 30min to send this PR for fun? Next time I'll just open the issue and let you deal with it.

Believe it or not, there is an issue with these images.

My help stops here.

image

@Hugoo

Hugoo commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

image

image

https://docs.dappnode.io/user-guide/ui/recommended-set-ups/add-vpn-devices/

image

https://docs.dappnode.io/user-guide/img/add_wireguard_device_3.png

image

image

One half of the page loads images from ../../../../img/ and the other half from ../../../img/ - i'm curious how this can work?

Not ok:
image

Ok:
image

@alexpeterson91

Copy link
Copy Markdown
Contributor

Wow, I apologize, seems you misunderstood me.
And I see now this is your first contribution with us.
Welcome!
I'm very sorry for the miscommunication, I'm to blame I should Have seen your tag as first time contributor here, and also should have included a very important bit of information that led to my request for changes. Before submitting PRs to any of our repos please first either use choose and link an existing issue with details on the issue and how to reproduce it, including screenshots of the issue along with the system info and software versions how things are being accessed and what browser is used. All of us us on the team including the core devs do this so that things are properly documented and can be easily reproduced to keep a clear record and trail of source changes, and why they were made.

I reviewed the current docs site and the deployed Netlify link with your fix and found them to be completely identical on both Brave and Chrome browsers, which are the most used browsers by our community. Without literally any context of the issue aside from just a partial screen grab there wasn't much to go on to try to reproduce it. I don't know why my review didn't include a comment asking you to make an issue to link to or at least ask for more context since 2 attempts to reproduce the issue, failed for me during my review thus request for changes, but it's totally on me for not adding that comment, not sure if i just blanked and forgot as I was doing something else at the same time, or if something bugged out, but anyways I apologize we got off on the wrong foot, once again Welcome to the community!

I'll edit my previous comment with what i failed to include and will make an issue for this if you would like, since it's more widespread than your fixes from whatI've seen so far but could also be a totally different issue.

@Hugoo

Hugoo commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

All good thanks, I'll open an issue ;)

@alexpeterson91
alexpeterson91 merged commit 849b434 into dappnode:master Apr 5, 2022
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.

2 participants