Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed broken discussion list pagination when clicking on the update button #33

Merged
merged 2 commits into from
Dec 12, 2021

Conversation

rafaucau
Copy link
Contributor

@rafaucau rafaucau commented Dec 11, 2021

Fixes #3111

Changes proposed in this pull request:

This PR fixes a bug with broken pagination after clicking on the update button.
More about the issue: flarum/framework#3111

Can this be released as 1.1.1, please?

Screenshot

2021-12-11.04-38-29.mp4

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

@rafaucau
Copy link
Contributor Author

BTW when I looked in the code for this extension, it is very outdated. As the next PR I might update and improve it. What do you think about it?

@dsevillamartin
Copy link
Member

Is the fix removing the false attr by itself or also changing it to app.discussions? Pretty sure we don't want to do the last part for extensibility - I think we switched some things in core to use the attr states recently as well.

@rafaucau
Copy link
Contributor Author

rafaucau commented Dec 11, 2021

@datitisev Oh, after removing false it also works. I've corrected it.

@askvortsov1
Copy link
Member

Thank you for the PR! This makes sense, not sure why it was false to begin with; this type of error is exactly what TypeScript should help prevent.

BTW when I looked in the code for this extension, it is very outdated. As the next PR I might update and improve it. What do you think about it?

That would definitely be welcomed! Any particular ideas for changes / improvements?

@askvortsov1 askvortsov1 merged commit b8ef3ba into flarum:master Dec 12, 2021
@rafaucau rafaucau deleted the fix_broken_pagination branch December 12, 2021 15:02
@rafaucau
Copy link
Contributor Author

@askvortsov1 Any particular ideas for changes / improvements?

  • Convert to Typescript
  • Update dependencies, e.g. pusher-php-server
  • Maybe it would be better to bundle pusher-js into this extension instead of using getScript?

  • Add information in the discussion that someone is currently writing post. It is useful in hot discussions

@askvortsov1
Copy link
Member

@askvortsov1 Any particular ideas for changes / improvements?

  • Convert to Typescript

  • Update dependencies, e.g. pusher-php-server

  • Maybe it would be better to bundle pusher-js into this extension instead of using getScript?

  • Add information in the discussion that someone is currently writing post. It is useful in hot discussions

These all seem like great improvements, thank you very much!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Pusher] Discussion list pagination breaks when clicking on the update button
4 participants