Hello,
let’s say I link to a category: Contribute > Feature
The topic map top link will read /c/feature or something like that. It would be nicer to have a title like Feature Category.
PS: I noticed this in a private message.
The discussion revolves around improving the topic map top link title for links to categories in Discourse. rriemann suggests changing the title from /c/feature to Feature Category. erlend_sh is unsure what the “topic map top link” refers to, and rriemann clarifies its location.
kasiabulat offers to work on the issue as a starter task, but sam notes that the fix is more complicated than expected, especially with localization in mind. sam closes the pull request and suggests that kasiabulat tackle the issue on the server side.
codinghorror proposes suppressing category links from the topic map, but sam thinks this might not be the best solution. Instead, sam suggests that the issue lies in the fact that internal links don’t have a title, which is needed for the topic map.
kasiabulat wants to continue working on the issue and asks for guidance. sam advises changing the category-hashtag.js.es6 file and notes that tests will need to be updated. kasiabulat proposes a solution, but realizes that it only partially solves the problem and asks for further guidance.
Hello,
let’s say I link to a category: Contribute > Feature
The topic map top link will read /c/feature or something like that. It would be nicer to have a title like Feature Category.
PS: I noticed this in a private message.
I don’t understand what the “The topic map top link” is
Aaah okay. Yeh changing that title might make make sense.
Hi! I’m new to Discourse and it is the first open source project I’m working on. Can I take this issue as my starter-task? I think it will help me to get to know the project.
Sure, go for it
Do not hesitate if you need help.
Thank you! I found the place, which needed a change, edited the code and created a pull request: UX: better topic map top link title for links to categories by kasiabulat · Pull Request #5289 · discourse/discourse · GitHub.
Hi @kasiabulat I was just reviewing this and sadly this is bit complicated, especially when localization is considered.
The bug here is the the title field in TopicLink is not being populated for these internal category links, the fix is fully server side not client side.
I did remove #starter-task from here cause it is somewhat complicated, but if you feel like giving it a shot again on the server side let me know … so sorry I need to close the open PR I feel extra bad about it. We should have guided you properly.
I would just suppress category links from the topic map. I don’t think they add a lot. That would also be an easier PR I think @sam?
Marginally, cause only easy fix is to suppress all internal links, you don’t really want to write yet another route matcher client side.
We could suppress all internal links that don’t have a title, that would be somewhat easier, but then what if you link to TOS or something else that is important.
Lets test this out: https://meta.discourse.org/tos, Privacy policy | Discourse - Civilized Discussion, /faq
Hmmm: yeah
<a class="hashtag" href="/c/2-feature">#<span>feature</span></a> yeah this thing needs a title which our CommonMark engine should provide and then a proper fix would flow.
Wow this is pretty messy for internal links:
looks like mini onebox is not setting title…
Yeah simply adding title for internal links would be best there. I think you meant to say ‘topic map’ in your last sentence as well?
OK, so I think I would like to coutinue working on it. Can you give me some tips on where can I begin and somehow summarize what should be done?
@eviltrout may be able to advise he worked on it last.
Simplest change is to amend https://github.com/discourse/discourse/blob/master/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js.es6 it is already covered in tests, so they will need changing
Thanks for the tip. Well, I could change the line 11: token.attrs = [['class', 'hashtag'], ['href', result[0]]] (in https://github.com/discourse/discourse/blob/master/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js.es6), so that there is also title added as an argument (I guess that there is a funtion that changes slug to the category name, but I haven’t found it yet). If I do it that way, hashtags to new created categories (not all of them, so I think it doesn’t satisfy us) will look something like: <a class="hashtag" href="/c/1-category-name" title="Category Name">#<span>category-name</span></a> in post replies. And then what will be needed is getting this title out of the attributes and showing it the post summary (topic map). But the title from attributes is null for these hashtags in the place I tried to change in my previous pull request (discourse/app/assets/javascripts/discourse/widgets/topic-map.js.es6 at 3b7128102c12d72d0303c83e4f113aa06c23655c · discourse/discourse · GitHub, call of the function createWidget creating topic-map-link widget, line 127), so this title is probably changed by some other functions earlier and for now I don’t know how exactly it flows. Should I dig into it and analyze how it all works? And please correct me if I don’t undestand something in what I described.