Repository navigation
Manchester | 26-SDC-Jun | Fithi Teklom | Sprint 2 | Rebloom Bloom - #300
Fithi-Teklom wants to merge 5 commits into
Conversation
illicitonion
left a comment
There was a problem hiding this comment.
This generally looks ok, but quite a few things don't seem quite right in the UI.
I signed in and rebloomed something. The page refreshed, but nothing changed. There was no indication my rebloom was successful. The original bloom didn't seem changed at all. If I try re-blooming it again, the button updates to say "Already re-bloomed".
I looked at my home timeline, the timeline of my user, and the timeline of the user who made the original bloom. None seemed changed at all by the fact that I'd done a rebloom.
Please give some thought to these and fix up your implementation so what's going on is more obvious to the user.
There was a problem hiding this comment.
You've added this file, which is useful, but it doesn't currently get added to the database.
You should either update the existing scripts to run this file, or add an entry a README or similar explaining how to apply it.
There was a problem hiding this comment.
Thanks for the feedback. Iβve improved the re-bloom success message, fixed the home and profile timelines to display re-blooms, and added documentation explaining how to apply the database migration.
illicitonion
left a comment
There was a problem hiding this comment.
This generally looks good, but a few things still to consider, as well as the specific comments:
- When I open a page with a bloom I've already rebloomed, I see a "Re-bloom" button that will never work. Can you avoid showing people buttons they can't actually do/click?
- I don't see reblooms in the home timeline, only in a user's timeline
| user_blooms = blooms.get_blooms_for_user(profile_username) | ||
| user_blooms.reverse() | ||
| return jsonify(user_blooms) | ||
| user_reblooms = blooms.get_reblooms_for_user(profile_username) |
There was a problem hiding this comment.
All of this code looks exactly the same as the code in other_profile - how could you avoid this duplication?
| setTimeout(() => { | ||
| window.location.reload(); | ||
| }, 1000); |
There was a problem hiding this comment.
We generally avoid reloading the whole page - can you cause a re-render without reloading the whole page?
Self checklist
Task code
CYF-1171
Changelist
-Added re-bloom functionality to allow users to re-bloom existing blooms.
-Updated the home timeline to display re-blooms and attribution.
-Prevented duplicate re-blooms by the same user.
-Fixed bloom content text wrapping in the timeline.
###Testing
-Verified re-blooms appear in the home timeline.
-Confirmed duplicate re-blooms are handled correctly.
-Checked that long bloom content wraps onto multiple lines.