This is a draft because I am still figuring out where to put the feature topic info on the User route
You’ve signed the CLA, markvanlan. Thank you! This pull request is ready for review.
The user card right now has all its rows numbered (from first-sixth), but all the rows following the first and second are styled no differently. I needed to add another row, and this structure made it so annoying to arbitrarily increase the class names from “third” to “fourth” and so on.
This change does not alter the user card appearance, until the user “features a topic” on their profile
The title of this pull request changed from “FEATURE: Featured topic on user card” to "FEATURE: Featured topic for user profile & card
Yeah moving away from the numbered rows makes sense…
I suspect that some sites use those numbered classes to hide specific sections of the user card, but it looks like the children of those divs can be used to the safe effect? Instead of hiding
.location-and-website should work the same way (most of the time anyway).
@awesomerobot Yeah, all those rows have classes like
.location-and-website, and I would be shocked if themes would access those elements by row rather than that class.
If you created the topic you are viewing, this
Feature On Profile button will now appear.
Featured Topic link appears in the user card.
As well as the user’s profile (this is the view logged in as someone else. This view looks different if you are viewing yourself, and does not show the link.)
And finally, users can see their featured topic under
preferences -> profile
Is there a bit more context where this feature is discussed?
basically people would like to feature a particular forum topic on their user card
Got it, maybe lets discuss the UX a bit more on meta, I feel like the full title + the text “featured topic:” is mega noisy in then featuring scenario.
In some ways when you feature stuff like this you probably want to select a font awesome icon (with a default) and then a few short words…
Then we can display this link next to website on that line.
Will post this on meta as well.
When this error has failed in the past, the error is because the test itself broke; it did not say the error message it was supposed to. It was the cannot modify frozen string error.
I believe this is unrelated and can be safely ignored.
A nice start! Let’s keep iterating on it!
We should use
I don’t think this needs to be a closure since it’s not wrapping any variables in this scope. We’d use less memory by making it a method on the controller like
This finally doesn’t seem to be doing anything? Could be removed.
Also we should make sure this topic isn’t in a restricted group.
We’re going to need more checks on the topic here. I suggest a
can_feature_topic guardian method because we’ll want to make sure:
- The topic is 100% public (not a PM, not in a secure group)
- The user created the topic
Otherwise I could hit this API endpoint and feature a topic that is private!
We prefer to return
success_json from methods rather than an empty body.
where(featured_topic_id: topic.id).update_all(featured_topic_id: nil)