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.

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

Documentation changes related to context code snippets #3396

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

imbus64
Copy link
Contributor

@imbus64 imbus64 commented Sep 8, 2023

The way the documentation demonstrates context usage does not work for me (panics on expect).
Its unclear if this is because im running a BrowserRouter in between the context provider and consumers. Either way, adding the Rc makes it work as expected.

This change just concerns the 0.20 version of the docs.

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Visit the preview URL for this PR (updated for commit c2fb476):

https://yew-rs--pr3396-master-307iqmoi.web.app

(expires Fri, 15 Sep 2023 03:05:15 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Member

@cecton cecton left a comment

Choose a reason for hiding this comment

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

Actually not sure if this is a bug that should be addressed later on or not.

@hamza1311 what do you think?

In any case, the code in the documentation should be working and if this works we should merge this right now (otherwise it's terrible for the users). We can add a comment somewhere or open an issue to fix this one later (if this is actually a bug).

@cecton cecton mentioned this pull request Sep 23, 2023
2 tasks
@futursolo futursolo merged commit 0c802f8 into yewstack:master Sep 23, 2023
4 checks passed
@ranile
Copy link
Member

ranile commented Sep 23, 2023

Actually not sure if this is a bug that should be addressed later on or not.

This was a bug... in the code snippet. The types need to be same. It Rc'd later on but not at call site

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.

4 participants