zulip-flutter icon indicating copy to clipboard operation
zulip-flutter copied to clipboard

internal_links: Add "/" before fragment identifier

Open satyam372 opened this issue 1 year ago • 6 comments

Fixes: #845

This adds a '/ ' after hostname. when this Url is posted in Zulip message it gets linkified. Screenshot_20240809-213108 1

satyam372 avatar Aug 09 '24 16:08 satyam372

Thanks for the PR! Left some comments.

PIG208 avatar Aug 12 '24 18:08 PIG208

PTAL @PIG208

satyam372 avatar Sep 23 '24 18:09 satyam372

PTAL @PIG208 And thank you for your suggestion.

satyam372 avatar Sep 26 '24 19:09 satyam372

Thanks @gnprice and @PIG208 for the review. just a small question.
In the current tests, we are dynamically generating and verifying the URL. Should we be comparing the generated URL directly against a hardcoded, literal full URL string within the test (e.g., including specific realm URLs and message IDs), rather than dynamically building it?

satyam372 avatar Oct 01 '24 18:10 satyam372

Yes. As the issue says:

we should also add a few tests that include, verbatim, the full URL they expect as the output.

For further questions about how to complete the PR, I recommend a chat thread in #mobile-dev-help. That may get a faster response.

gnprice avatar Oct 04 '24 21:10 gnprice

PTAL @PIG208

satyam372 avatar Oct 11 '24 07:10 satyam372

PTAL @PIG208

satyam372 avatar Oct 17 '24 19:10 satyam372

Please review @gnprice.

satyam372 avatar Oct 22 '24 06:10 satyam372