Sheffield | 25-SDC-Nov | Sheida Shabankari | Sprint 1 |Hashtag slowing down browser#103
Conversation
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/tests/hashtag.spec.mjs
Outdated
| // When I navigate to the hashtag | ||
| await page.goto("/#/hashtag/do"); | ||
| // And I wait a reasonable time for any additional requests | ||
| await page.waitForTimeout(200); |
There was a problem hiding this comment.
Waiting for fixed periods of time is a recipe for flakiness in tests - prefer to wait e.g. for the title of the page to be updated, or for some blooms to be displayed, or something similar.
There was a problem hiding this comment.
I replaced the fixed timeout with a wait for the API response to ensure the test only continues once the hashtag data is loaded. This avoids flakiness and ensures the test reliably reflects the UI state.
|
|
||
| const normalizedHashtag = hashtag.startsWith("#") ? hashtag : `#${hashtag}`; | ||
| if (state.currentHashtag !== normalizedHashtag) { | ||
| state.currentHashtag = normalizedHashtag; |
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.
Thanks for the feedback — I understand the concern about showing the title for a new hashtag while the content might be from a previous hashtag if the API response hasn't returned yet.
To handle this, the code now checks that the hashtag in state is still current before using the API response, so old data is ignored and the view remains consistent.
Summary
This PR fixes a bug where clicking on a hashtag caused the page to repeatedly refresh and flash blank.
Cause
The hashtagView was calling apiService.getBloomsByHashtag multiple times due to re-renders.
Because the hashtag value was not normalized or tracked correctly, the view kept triggering new API requests, resulting in an infinite loop.
Fix
The hashtag is now normalized (always prefixed with #) and stored in state.currentHashtag.
The API request is only made when the hashtag actually changes, preventing repeated calls and re-renders.
Tests
A new Playwright test was added to ensure the hashtag endpoint is called only once and does not enter an infinite request loop.