Skip to content
This repository has been archived by the owner on Nov 21, 2020. It is now read-only.

Fix ads between posts #2

Merged
merged 2 commits into from
Feb 27, 2017
Merged

Fix ads between posts #2

merged 2 commits into from
Feb 27, 2017

Conversation

clarkwinkelmann
Copy link
Contributor

After many trials and errors on the trail of @luceos, I managed to find a solution that satisfies most (all ?) of the problems we had:

  • Posts order is maintained when posting/editing
  • The scrubber is happy and reflects current post
  • The url reflects the current post id (not something from the ad at the top of the screen)

The following solutions were tested without success:

  • override PostStream.posts()
  • override Discussion.postIds()

It looks like something is reordering the posts after the Mithril object is returned from PostStream.views(), probably based on the content of data-index, data-number or data-id. Given the way PostStream gives these numbers out there is no way to push an item with a valid, yet non-duplicated value of index/number/id without forcing to change all the numbers from all posts until the end of the discussion.

The current solution also works if we push ads items directly in the stream (they don't get reordered randomly). However, this breaks the PostStreamScrubber.

@luceos
Copy link
Contributor

luceos commented Feb 27, 2017

@clarkwinkelmann first, could you create an issue on @flarum/core? Feel free to relate back to this PR.

I will review the implementation today and merge if ok. Thanks for the investigation it's good to see you ran into exactly the same issues I had 😄

@luceos luceos merged commit 98747ba into master Feb 27, 2017
@luceos luceos deleted the fix-between branch February 27, 2017 09:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants