Skip to content

Adding spaces on paragraph endings for search results #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

kelvinst
Copy link
Contributor

Fixes #1164

@kelvinst
Copy link
Contributor Author

So this should be enough to fix it, could not find any test for the search templates, can you point me where I can implement the tests for it? Or is the tests I added enough already in your opinion?

@josevalim
Copy link
Member

I guess before when we generated the html from markdown, we would automatically add a newline between paragraphs. But now we don't. This fix looks good to me but maybe @wojtekmach prefers to fix it on the HTML generation side. Thoughts?

@kelvinst
Copy link
Contributor Author

kelvinst commented May 17, 2020 via email

@wojtekmach
Copy link
Member

Yeah, I think it would be nicer to fix it on the html generation side, I'll briefly look that but it's likely we'll just end up with this patch. :)

@davydog187
Copy link
Contributor

I will take a stab at fixing this on the HTML generation side.

@davydog187
Copy link
Contributor

@wojtekmach this can't be fixed on the HTML generation side.

"EnhancementsSeparate tasks into "Mix Tasks" in sidebarAdd types to the search resultsImprove accessibilityBug fixesStrip nesting HTML tags in h2 headersRemove the old search results after every new search attempt"

This is the value that the template is working with

@davydog187
Copy link
Contributor

Sorry, in my previous comment I misunderstood what was meant by HTML generation side (I think?)

Regardless, this PR still has issues

Screen Shot 2020-05-17 at 10 25 27 AM

@kelvinst
Copy link
Contributor Author

Well, that's odd, I didn't touch the actual tag regex, can you test the same with the old code? Maybe that's an old bug.

@davydog187
Copy link
Contributor

@kelvinst i think its due to the way your logic for adding spaces works

@michallepicki
Copy link
Contributor

michallepicki commented Nov 14, 2020

@davydog187 I pulled the branch and I'm not seeing the issue from your screenshot:
image

Or maybe I'm missing it, what is the issue exactly?

edit: To be clear, this PR seems to be working for me

@davydog187
Copy link
Contributor

@michallepicki its been a while, so I'm not sure.

@kelvinst
Copy link
Contributor Author

kelvinst commented Mar 29, 2021

Just revisiting this old PR, I have tested it and couldn't repeat @davydog187's problem either. But mine doesn't even show the link for sidebar, maybe that changed on the recent version, IDK 🤷‍♂️

Captura de Tela 2021-03-29 às 07 44 55

@josevalim josevalim merged commit fd55c41 into elixir-lang:master Mar 29, 2021
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@kelvinst kelvinst deleted the fix-1164 branch March 29, 2021 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Replace newlines with spaces in search data/results
5 participants