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

Fix various bugs on comment counters and pagination #133

Merged
merged 7 commits into from
Nov 30, 2023

Conversation

gkoscky
Copy link
Contributor

@gkoscky gkoscky commented Oct 6, 2023

This contains a series of small fixes addressing problems with the "comment counter" under posts in the blog

Comments

  • Badges saying "Leave a reply" for posts that already had comments
    We figure out if there are any comments on a post by querying the associated topic in our Discourse Collective. Unfortunately a bad CORS setting in Discourse was blocking the requests coming from the blog.
    Just needed to change the Access-Control-Allow-Origin setting from https://writings.flashbots.com to https://writings.flashbots.net

  • Comment counts were off by one
    Discourse doesn't really give you a direct way to get the number of replies under a Topic. We were using the posts_count, but that's the total number of posts and not replies so we needed to subtract one - the original post - from that count.

  • CSS fixes
    The button was disappearing in Dark Mode. The SVG bubble has better alignment. And link color is no longer changing on hover for Firefox.

  • Don't render the comment button if there's no Collective post
    The "Leave a Reply" button could be rendered if the post had a forum_link property set to empty. So now there's better handling of that case

Navigation

  • Change the cursor to a click-hand on navigation links
    Better UX, so the user knows they're clickable

  • Scroll up to first post on page change
    Previously changing the page would leave the page stuck at the bottom, so the user would have to scroll back up to see the posts and potentially have to scroll all the way back down to go to the next page.
    Make it so that the page automatically scrolls up when you change the page


Fixes: #127

Discourse is very peculiar when it comes to what it counts as a "reply", so in order to get a current, sensible count we must just count all posts and subtract one, for the original post.
The button was disappearing in dark mode
The alignment was also off
And the color was changing on hover in Firefox
If the `forum_link` property was present on a post but had no value, the "Leave a reply" button would be rendered but not be a link (since we have no href).

So let's check for any valid `forum_link` instead of just not undefined
The latest posts was missing a link to its discussion in the forums
So users know they're clickable
Being stuck on the bottom of the page is bad usability, so scroll all the way back up so users can check out the posts
@gkoscky gkoscky added the bug Something isn't working label Oct 6, 2023
@gkoscky gkoscky self-assigned this Oct 6, 2023
@vercel
Copy link

vercel bot commented Oct 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flashbots-writings-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 29, 2023 9:56pm

@gkoscky gkoscky changed the title #127: Fix various bugs on comment counters and pagination Fix various bugs on comment counters and pagination Oct 6, 2023
Copy link
Collaborator

@deadpine deadpine left a comment

Choose a reason for hiding this comment

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

LGTM!

@gkoscky gkoscky merged commit 2f427e8 into main Nov 30, 2023
3 checks passed
@gkoscky gkoscky deleted the gkoscky/fix-comment-bugs branch November 30, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reply counter is wrong on Posts
2 participants