Skip to content

feat: make it easier to add content after the first post #4186

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
merged 4 commits into from
Feb 14, 2025

Conversation

DavideIadeluca
Copy link
Contributor

2.x port of #4050

Progresses #4060

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

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.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

@DavideIadeluca DavideIadeluca changed the title feat(core): make it easier to add content after the first post refactor(core): make it easier to add content after the first post Feb 10, 2025
@DavideIadeluca DavideIadeluca marked this pull request as ready for review February 10, 2025 15:09
@DavideIadeluca DavideIadeluca requested a review from a team as a code owner February 10, 2025 15:09
<div className="PostStream-item" {...attrs}>
{content}
</div>
);

if (i === 0 && this.afterFirstPostItems().toArray().length > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the intent after the first post of the discussion, or after the first of the visible posts? because i === 0 will always be the first of the currently visible posts, not necessarily the first post of the discussion.

For example, if you are viewing posts 200 - 220 of a discussion, the 200th is the first (i === 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that was most certainly the intention. Pushed a change which works much better now

@DavideIadeluca
Copy link
Contributor Author

DavideIadeluca commented Feb 14, 2025

@SychO9 seems like phpstan/phpstan v1.12.18 (released yesterday) has broken CI here. Might you be able to look into this? Don't think it's a GitHub quirk this time around

Copy link
Member

@SychO9 SychO9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


Don't worry about the phpstan failure, will look into that separately.

@SychO9 SychO9 changed the title refactor(core): make it easier to add content after the first post feat: make it easier to add content after the first post Feb 14, 2025
@SychO9 SychO9 added this to the 2.0.0-beta.3 milestone Feb 14, 2025
@SychO9 SychO9 merged commit 9758592 into flarum:2.x Feb 14, 2025
18 of 21 checks passed
@DavideIadeluca DavideIadeluca deleted the di/after-first-post-item branch February 14, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants