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

fix(Page): Use "open" prop #135

Merged

Conversation

aDogCalledSpot
Copy link
Contributor

No description provided.

@ctron
Copy link
Member

ctron commented Feb 19, 2024

I am wondering if the page should control this itself? Or maybe just honor the state of the property. Or, add an optional callback, relying on the callback when set, and self-manager otherwise?

@aDogCalledSpot
Copy link
Contributor Author

The silent overwriting of the prop is definitely a bit strange.

How about using Option<bool> for the PageSidebar.open? Then check for the prop in the Page. If it's Some(b) then use it and if it's None then self-manage.

@ctron
Copy link
Member

ctron commented Feb 19, 2024

Would work too. But I guess we would still need a callback for propagate this state.

@aDogCalledSpot
Copy link
Contributor Author

I'll merge this for now

@aDogCalledSpot aDogCalledSpot merged commit 6366d67 into patternfly-yew:main Feb 20, 2024
2 checks passed
@aDogCalledSpot aDogCalledSpot deleted the fix_sidebar_use_open_prop branch February 20, 2024 14:01
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.

None yet

2 participants