downforacross / downforacross.com

Web frontend for downforacross.com -- continuation of stevenhao/crosswordsio
https://downforacrosscom.downforacross1.now.sh
MIT License
220 stars 92 forks source link

Added function in ClueText to determine whether text should be italic… #295

Closed zhiyi-zhang-duke closed 6 months ago

zhiyi-zhang-duke commented 7 months ago

This code change adds a shouldItalicize function as well as changes ClueText.js into a react component. If regex matches twice, we know that the text needs to be italicized. Here's a screenshot of the new component in action: image

sweep-ai[bot] commented 7 months ago

Apply Sweep Rules to your PR?

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
downforacross.com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2023 1:00am
zhiyi-zhang-duke commented 6 months ago
  1. can we keep this as function component instead of a class component?

Wow I totally missed it was already a functional component. Sorry about that, it's restored!

  1. i think there are a lot of false positives for clues that start & end with quotation marks, so as written i don't think the behavior is strictly correct. Perhaps we can change the rule to checking if the clue text is encased by 2 quotes (""x"" instead of "x")?

Great point, I made the regex do exactly what you said and only check for two double quotes at the beginning and end of the string.

  1. since there is already some logic involving parsing out <i> tags for italics, maybe we should avoid mixing the control flow with that? e.g. if entire clue is italic: ... else parse out <i> parts

Ah I actually glossed over that logic instead of really trying to understand it. 😅 I incorporated the control flow you suggested.

stevenhao commented 6 months ago

great!