Skip to content

Fix new post injected above unread sticky #1868

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

Merged

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Sep 4, 2019

Fixes #1751

Changes proposed in this pull request:
instead of prepending the new discussion to the list, this refreshes the list

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run php vendor/bin/phpunit).

@SychO9 SychO9 closed this Sep 5, 2019
@SychO9
Copy link
Member Author

SychO9 commented Sep 6, 2019

I had some issues on my end, but I was able to install pusher, from what I can see, it only works when a reply to an existing discussion is submitted, a new discussion does not trigger anything so I'd say the relevant code in the extension can be removed

I have however restored the addDiscussion method

@stale

This comment has been minimized.

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Jan 18, 2020
@franzliedke
Copy link
Contributor

franzliedke commented Feb 7, 2020

I'll review this in time for the upcoming beta.12.

@stale stale bot removed the stale Issues that have had over 90 days of inactivity label Feb 7, 2020
@askvortsov1
Copy link
Member

I'll review this in time for the upcoming beta.12.

I can confirm that this fixes the issue, so unless there's code style problems we can probably merge this.

@franzliedke franzliedke merged commit 30942bd into flarum:master Apr 3, 2020
@franzliedke
Copy link
Contributor

@SychO9 Congrats for getting your first PR merged! 🙌

And sorry it took so long. 😕

@SychO9 SychO9 deleted the sycho/1751-fix-new-discussion-inject branch April 3, 2020 21:06
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.

[sticky] New post injected above unread sticky
3 participants