Skip to content

Conversation

laomuon
Copy link

@laomuon laomuon commented Nov 9, 2023

What does this PR do, and why?

Make the external link in message info pop-up to be opened in the browser when hit Enter

How did you test this?

  • Manually - Behavioral changes
  • Manually - Visual changes
  • Adapting existing automated tests
  • Adding automated tests for new behavior (or missing tests)
  • Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

  • It is a minimal coherent idea
  • It has a commit summary following the documented style (title & body)
  • It has a commit summary describing the motivation and reasoning for the change
  • It individually passes linting and tests
  • It contains test additions for any new behavior
  • It flows clearly from a previous branch commit, and/or prepares for the next commit

@zulipbot zulipbot added the size: S [Automatic label added by zulipbot] label Nov 9, 2023
@laomuon laomuon force-pushed the open-link-in-browser branch from fd3ab98 to 50fb7a0 Compare November 9, 2023 23:08
@laomuon
Copy link
Author

laomuon commented Nov 9, 2023

@zulipbot add "area: UI" "area: event handling" "PR needs review"

@zulipbot zulipbot added area: event handling How events from the server are responded to area: UI General user interface update PR needs review PR requires feedback to proceed labels Nov 9, 2023
feat: Open in browser for website links in msgs

reformat: reformat with black
@laomuon laomuon force-pushed the open-link-in-browser branch from 50fb7a0 to 553f3e9 Compare November 9, 2023 23:16
@neiljp neiljp removed the area: event handling How events from the server are responded to label Nov 10, 2023
Comment on lines +472 to +473
elif self.link.startswith(("https://", "http://")):
self.controller.open_in_browser(self.link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be useful to define the protocols we support somewhere as a constant rather than inline here.

@neiljp neiljp added this to the Next Release milestone Nov 11, 2023
@mounilKshah
Copy link
Collaborator

Thanks @laomuon for working on this. I tried it and it worked well on my system. In the current state of the PR, I don't have much to add to what Neil has already suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI General user interface update PR needs review PR requires feedback to proceed size: S [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants