Hello,
let’s say I link to a category: 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.
Hello,
let’s say I link to a category: 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: https://github.com/discourse/discourse/pull/5289.
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, https://meta.discourse.org/privacy, /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 (https://github.com/discourse/discourse/blob/3b7128102c12d72d0303c83e4f113aa06c23655c/app/assets/javascripts/discourse/widgets/topic-map.js.es6, 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.