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

Map over display function instead of displayed value. #1031

Merged
merged 1 commit into from May 15, 2020

Conversation

frasermince
Copy link
Contributor

Previously we were mapping over the displayed status boolean and
treating it like a promise when it wasn't one. I assume we meant to
instead call the display function. I discovered this because I saw on mobile the relocated event wasn't being called when navigating to a chapter that had not yet been loaded. This fixes that issues and makes the relocated event always be called on chapter change.

Previously we were mapping over the displayed status boolean and
treating it like a promise when it wasn't one. I assume we meant to
instead call the display function.
@fchasen
Copy link
Contributor

fchasen commented May 14, 2020

Thanks for find this!

Still looking into the promise as something funky is, as you mentioned displayed is just a boolean and it seems to be looking from the promise result of display()

return displayed.promise;

I don't think we wan't to be calling display here though, since that will trigger a render and this is just looking to know when all the views have finished displaying.

Looking at this, I think a solution could be to add a manager.displayed = [<Promise>] that keeps the results of

return view.display(this.request);

@fchasen
Copy link
Contributor

fchasen commented May 15, 2020

On testing some more, my suggestion didn't work at all... will need to tidy this up a bit eventually, but this change works well for now!

@fchasen fchasen merged commit 20f5ea5 into futurepress:master May 15, 2020
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