Skip to content
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

Do not replace timeskip when it is in a URL #447

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

wtfparadox
Copy link
Contributor

@wtfparadox wtfparadox commented Oct 21, 2022

Description

A bug occurs in the show notes when a URL contains a text formatted like a timeskip. In this scenario, the text should not be replaced, as it will break the URL.

Fixes #145

What is done

I wrote a method that checks whether a potential timeskip is in a URL by finding the nearest HTML closing tag to the potential timeskip.
I have also written a new test that checks the exact string the issue occurred on.

Checklist

  • In "Rebuild - Tatsuhiko Miyagawa" episode 325 text formatted like timeskip is not replaced by timeskip
  • In "Foundation - True Ventures" episode 8, timeskips work properly
  • Should this change be included in the release notes? If yes, please add a line in CHANGELOG.md
  • Have you tested in landscape?
  • Have you tested accessibility with TalkBack?
  • Have you tested in different themes?
  • Does the change work with a large display font?
  • Are all the strings localized? I havent touched any string resources
  • Could you have written any new tests?
  • Did you include Compose previews with any components? I havent used compose

Video:

timeskipinurlfix.mp4

@wtfparadox wtfparadox requested a review from a team as a code owner October 21, 2022 20:08
@CLAassistant
Copy link

CLAassistant commented Oct 21, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@geekygecko geekygecko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great change and including a test. It's really appreciated. 🙏

@geekygecko geekygecko merged commit a99ca65 into Automattic:main Oct 21, 2022
@wtfparadox wtfparadox deleted the fix/timeskip-in-url branch October 23, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URLs in link text with xx:yy format in Notes will be replaced as time skip
3 participants