Markdown support in beatmap discussions

peppy commented 8 years ago

Mod comments should be able to use markdown. Possibly with headings disallowed.

Ephemeralis commented 5 years ago

I'm told that markdown support for moddingv2 is currently in, but not enabled?

The full ensemble doesn't need to be in, but the basics like bold, italics, underline, strikethrough, external links, bullet and numbered lists would be a huge bonus to usability for those used to the syntax.

NoffyNoffeh commented 4 years ago

From what I can tell, markdown suppport is also present in reviews. Having it in part of the modding panel features and not the other seems inconsistent and makes this all the weirder. Being able to embed images which would display small but could be clicked to display large size in a gallery format (how it's handled for forum posts/ beatmap descriptions right now) would be especially useful for modding instead of always needing to click a link opening a different page/website to view visual suggestions. It feels like this is really a key feature that we've learned to do with out but is still sorely missing.

Related issue https://github.com/ppy/osu-web/issues/6082 also discusses this a bit

NoffyNoffeh commented 3 years ago

I'd like to get an update on this issue if possible. It's really wrong it's only partially present, by being in reviews and in beatmap comments, yet is still missing from modding posts themselves after nearly 5 years when it would be so useful.

peppy commented 3 years ago

Going to add this to the next milestone. We should definitely get the basic style codes in at very least, if not full support (minus headings as suggested).

nanaya commented 3 years ago

this will be fun in term of not killing the browser rendering hundreds of markdown

peppy commented 3 years ago

Is it the parsing step that is going to add overhead? Wondering if we just hack in a very simple implementation to get just the basic formatting working (italic / bold).

We can't just render on-screen elements because the post-render height is being used for the overall page layout, right?

NoffyNoffeh commented 3 years ago

I'm not technically well versed but how is it it works for comments but would have rendering issues for modding posts? Is there a key difference between them causing pain points?

nanaya commented 3 years ago

Comments are paginated but modding discussions may have hundreds of posts loaded at once.

NoffyNoffeh commented 3 years ago

Would paginating modding posts in a similar method as how beatmaps in search are loaded be possible?

VINXIS commented 3 years ago

auto refresh/infinite scroll as u go up/down the different sections in the modding page would be good for the modding discussion page no? Probably related to https://github.com/ppy/osu-web/issues/6285

peppy commented 3 years ago

As soon as you start using placeholders, it means you can't use browser search to search the page for text. This is one of the main reasons we haven't considered this until now. Would that not be considered an issue for modding?

NoffyNoffeh commented 3 years ago

With the modding and status filters available as well as visually seeing the timeline for each diff, browser search is not really needed. The main issue I can imagine is how it would relate to long discussions where 1 post has many many many replies. That is a case where ctrl+f is useful and I'm not sure how those posts would be loaded.

peppy commented 3 years ago

I guarantee even if you are happy with the functionality being removed, there will be people that depend on it. Also, this doesn't solve an equally hard issue of knowing how tall each post is (to get the correct page height), assuming we want to have the page height be correct from initial load and not completely break the UX.

NoffyNoffeh commented 3 years ago

In that case would adding functionality to search map discussions the same way you can search specific forum threads be an idea?

Not sure about the details for the UX issue though.

peppy commented 3 years ago

Yes, that is possible. It's also a much larger implementation, involving potentially new infrastructure to handle it. Now a simple change of adding markdown has spiraled into a potential several-month dependency chain.

Have to be sure that it's important enough to consider making such large changes. Also how such a change would slot in with the pending design changes we have lined up for the system (I know the discussions view is at least one version behind flyte's latest designs).

Okorin commented 3 years ago

Would only enabling rendering markdown stuff on the reviews tab make this idea more feasible or is that too whack?

The reviews generate separate posts on the timeline, but at least including pictures and whatnot between issues / some sort of formatting would already be cool

Edit: Reviews already allow B/ I, the most important thing for me when modding is not having to leave the page whenever i want to look at an image accompanying a suggestion which is currently impossible

NoffyNoffeh commented 3 years ago

I see. Is there somewhere to view future design changes and how they're intended to work to get a better idea of the plan?

Having markdown especially for allowing image embeds is very, very important. If that could at least be added to reviews it would be a good step forward and reviews have the basic markdown support so far already.

Reviews already have bold/italic for the paragraph parts, just posts don't.

However I'm not sure putting it the main thing for markdown on posts behind other steps like more search etc. is the best choice either when they're not uh, directly related. Which would leave it to that tradeoff of if people value markdown or ctrl+f more, from what I understand. Would a survey on that be helpful?

peppy commented 3 years ago

@nanaya as per https://github.com/ppy/osu-web/issues/451#issuecomment-801231558, what's the difficulty level of getting markdown support only in the reviews section? i know that uses its own editor and database structure, so wondering if it's a starting point to get this in.

nanaya commented 3 years ago

it's already markdown internally but because the editor itself is a visual editor, it needs ui for actually styling or adding things to them (among other things)

Ephemeralis commented 2 years ago

Is there anything in particular holding this back for further implementation? Markdown has been sorely needed for a long while now, especially for features like image embedding, etc.

peppy commented 2 years ago

Is there anything in particular holding this back for further implementation? Markdown has been sorely needed for a long while now, especially for features like image embedding, etc.

see the posts above. it should explain that nothing is holding this back apart from developer time. to confirm: the main concern here is the ability to embed images? with the consideration that said images would still need to be hosted externally (and will eventually 404)?

Ephemeralis commented 1 year ago

This totally fell by the wayside, but yes, embedding images is critical to being able to properly articulate most mods/suggestions/change. Preserving these images indefinitely (for the sake of history) would be good, but is nowhere near important enough to delay having the feature entirely.

The closest analogue I can think of would be having to open GitHub suggestion diffs in an entirely new window inside PRs every time you wanted to see what was in them. Since people have to click image URLs to do this constantly, reviews are an arduous slog at the mercy of the user's tab management.

The NAT have mentioned recently in internal discussions that this continues to be a severe friction point with beatmap discussions use.

nanaya commented 1 year ago

In addition to the ui requirement above, there may be additional concern regarding limiting the display of the image (how big can they be, how many are allowed in single post, and are we going to allow collapsing the longer posts and how, among other things). Some of which probably require optimising the page first because otherwise it'll just get even slower.

peppy commented 1 year ago

Bumping this to priority zero as it is required to make the system usable, according to the NAT team.

peppy commented 1 year ago

@notbakaneko nanaya mentions that you're still working through this, can you give a status update? I think you may need to stop cleanup efforts and just make this happen at this point, since it's taking so long.

Proposals:

If client side markdown rendering becoming too slow is a concern, do it server side and send the html
If images pushing pages off-screen is a concern (or initial image content taking too long / too much bandwidth to load), do js level async image loading and remove any seek animations so the amount the page is pushed down is reduced to only stuff on the current screen.

notbakaneko commented 1 year ago

It's not as straight forward as just rendering markdown and sending it to the client; there are additional client-side parsers on the discussion page and it turns out these only work in plain text. There's already a markdown renderer being used for discussion reviews, except the plugins being used here also only work with the very limited set of markdown being rendered (reviews aren't actually markdown either)

Basically,

need to update react-markdown (out of date, API changes, hard to find documentation for the old format)
update the related dependencies used by react-markdown that we're also using directly (versions not compatible)
fix all the breakages from updates
fix parsing and rendering of our plugins

zoey-on-github commented 1 year ago

im sorry if this isn't the right place to say this but why is this tagged both "priority:0" and "low priority"? the tags seem to contradict each other

notbakaneko commented 1 year ago

markdown support has been added in #9843; requests for additional syntax should open new issues

ppy / osu-web

Markdown support in beatmap discussions #451