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

fix: put summary in <Static> #10

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: put summary in <Static> #10

wants to merge 2 commits into from

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Nov 14, 2019

Fixes #9

Not happy with this - neither the setImmediate or the passing of components around.

The former is because otherwise the summary was rendered twice, the latter due to me being tired :P

@jeysal @thymikee ideas on making this better?

@jeysal
Copy link
Member

jeysal commented Nov 14, 2019

Isn't useEffect supposed to run the things after the render was committed to the screen? Wouldn't expect setImmediate to be necessary.
As for the component, if prop drilling was the reason for not just instantiating it deeper down, use Context? 😅

@SimenB
Copy link
Member Author

SimenB commented Nov 15, 2019

Isn't useEffect supposed to run the things after the render was committed to the screen?

I'd think so, might be a bug in Ink

@SimenB SimenB force-pushed the summary branch 2 times, most recently from c49bea9 to 80b1f9f Compare November 15, 2019 11:07
@SimenB SimenB force-pushed the summary branch 3 times, most recently from 367d5b0 to 33e43d8 Compare February 26, 2022 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Summary is missing from CI
2 participants
@SimenB @jeysal