WestMidlands | SDC NOV 2025| Sara Tahir | Sprint 1| Hashtag Slowing Down my Browser#99
WestMidlands | SDC NOV 2025| Sara Tahir | Sprint 1| Hashtag Slowing Down my Browser#99SaraTahir28 wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
…orking. Also updated settings to get backend running
SaraTahir28
added
the
Needs Review
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
This looks good, and well done for adding a test for the bug! I left a couple of comments to think about.
front-end/views/hashtag.mjs
Outdated
| //Only fetch data if the hashtag has changed since the last render. | ||
| // This prevents infinite loops caused by re-renders or route updates. | ||
| if (state.currentHashtag !== formattedHashtag) { | ||
| state.currentHashtag = hashtag; // Store the new hashtag BEFORE fetching so the next render sees the update. |
There was a problem hiding this comment.
This is an interesting choice - isn't there a problem here that we may show the title for the new hashtag but the content for the new hashtag, if we render again before the API request finishes?
There was a problem hiding this comment.
This makes sense, I have changed the code so now we only update state.currentHashtag when the hashtag actually changes, and the UI doesn’t re‑render until the fetch comes back and updates state.hashtagBlooms.
There was a problem hiding this comment.
I don't see the change here?
illicitonion
added
Reviewed
…based wait using the first [data-bloom] element, Also Updated hashtagView to only update state.currentHashtag and trigger a fetch when the formatted hashtag changes.
SaraTahir28
added
the
Needs Review
illicitonion
removed
the
Needs Review
SaraTahir28
left a comment
SaraTahir28
left a comment
There was a problem hiding this comment.
Hi I have tagged you where I changed the function. :)
| apiService.getBloomsByHashtag(hashtag); // Retrieve all blooms associated with this hashtag from the API. | ||
| state.currentHashtag = formattedHashtag; | ||
| apiService.getBloomsByHashtag(formattedHashtag); | ||
| } |
There was a problem hiding this comment.
@illicitonion I have changged the function here in this commit.
There was a problem hiding this comment.
Hmm... I still see state.currentHashtag = formattedHashtag; on line 22 - was the point not to remove that line?
There was a problem hiding this comment.
From what I understood from your first comment is that I updated state.currentHashtag immediately but state.hashtagBlooms still contained the old blooms so there was a possibility of UI showing mismatched data.
I replace it with updating state.currentHashtag when the hashtag actually changes.
The fix was just to store the formatted hashtag so the comparison works properly and the UI only updates when the data actually changes.
if I removed this line state.currentHashtag = formattedHashtag state.currentHashtag would never get updated
There was a problem hiding this comment.
getBloomsByHashtag already updates state.currentHashtag when it gets results. And currentHashtag is currently read in views/hashtag.mjs for deciding what hashtag heading to show.
If we just let getBloomsByHashtag do the update, we make sure currentHashtag and hashtagBlooms are always in sync. If you manually update currentHashtag, then someone triggers a re-render, we'll show the new hashtag but the old blooms.
SaraTahir28
added
the
Needs Review
2 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
This PR fixes an issue where navigating to a hashtag page triggered multiple repeated API requests, causing unnecessary network load and slowing down the browser. The root cause was that the hashtag view re‑rendered with the same hashtag, and the fetch function was being called on every render.
A Playwright test from the prep section is included to ensure this regression never returns.
Questions
No Questions