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.

Sign up for GitHub

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

Jump to bottom

Update README.md #628

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Update README.md #628

merged 2 commits into from
Oct 13, 2023

Conversation

elboyran
Copy link
Contributor

Fixed links to TS tutorials

@elboyran
Update README.md
7844ff5 
Fixed links to TS tutorials
@elboyran elboyran requested a review from cwmeijer July 25, 2023 14:55
cwmeijer
cwmeijer requested changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@cwmeijer cwmeijer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There seems to be something wrong with the links. I can't spot the error in the md myself, but when I test the links, I end up at the icon image instead of at target notebook. Can you check this?
image

@elboyran
Copy link
Contributor Author

elboyran commented Aug 1, 2023

Yes, I had the same problem and coudn't fix it :-( That's why I was hooping someone else can spot the problem.

geek-yang
geek-yang reviewed Sep 5, 2023
View reviewed changes
Copy link
Member

@geek-yang geek-yang left a comment

Choose a reason for hiding this comment

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

Yes, I had the same problem and coudn't fix it :-( That's why I was hooping someone else can spot the problem.

I suspect that this is due to the server side rendering trick of github. Currently the image is requested via [<img width="25" alt="Weather Logo" src="https://github.com/dianna-ai/dianna/assets/3244249/3ff3d639-ed2f-4a38-b7ac-957c984bce9f">], which fetches image from assets and ends up with rendering, with the link to the notebook unresolved.

To fix this, I guess we need to use the raw link to the image, rather than via assets (might be relevant with webpack thingy? apologize for my limited web knowledge 😓), for instance src="https://user-images.githubusercontent.com/3244249/242001499-3ff3d639-ed2f-4a38-b7ac-957c984bce9f.png"

@geek-yang
Copy link
Member

geek-yang commented Sep 5, 2023 •
edited
Loading

@elboyran Sorry for pushing commits to your PR (the easiest way for testing it 😂). Now it seems to work!

@cwmeijer cwmeijer self-assigned this Oct 3, 2023
cwmeijer
cwmeijer approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@cwmeijer cwmeijer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It works now! 🏆

Let's get it merged!

@cwmeijer cwmeijer moved this from To do to Ready for review in SS Sprint 1 - Pay off our technical debt Oct 3, 2023
@cwmeijer cwmeijer assigned elboyran and unassigned cwmeijer Oct 3, 2023
@elboyran elboyran merged commit e34891d into main Oct 13, 2023
14 of 18 checks passed
@elboyran elboyran moved this from Ready for review to Done in SS Sprint 1 - Pay off our technical debt Oct 13, 2023
@geek-yang geek-yang deleted the elboyran-patch-1 branch October 14, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geek-yang geek-yang geek-yang left review comments

@cwmeijer cwmeijer cwmeijer approved these changes

Assignees

@elboyran elboyran

Labels
None yet
Projects
No open projects
SS Sprint 1 - Pay off our technical debt
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@elboyran @geek-yang @cwmeijer