Skip to content

Conversation

@taooceros
Copy link
Member

@taooceros taooceros commented Jan 9, 2022

Fallback to direct process open when uri is not started with http/https

@taooceros taooceros changed the title Support Application Uri Support Application Uri in OpenUrl API call Jan 9, 2022
@JohnTheGr8
Copy link
Member

Maybe add a different overload that accepts a Uri for such cases? Having the inPrivate parameter there when this might not run in browser is awkward.

@taooceros
Copy link
Member Author

Maybe add a different overload that accepts a Uri for such cases? Having the inPrivate parameter there when this might not run in browser is awkward.

Hm if that's the case, I would doubt whether we should include a new public api because it is simply a helper function.

@taooceros taooceros added this to the 1.10.0 milestone Jan 12, 2022
@taooceros taooceros added the enhancement New feature or request label Jan 12, 2022
@JohnTheGr8
Copy link
Member

Hm if that's the case, I would doubt whether we should include a new public api because it is simply a helper function.

nothing about this suggests that you can pass an application Uri and it will be started as a process:

/// <summary>
/// Opens the url. The browser and mode used is based on what's configured in Flow's default browser settings.
/// </summary>
public void OpenUrl(string url, bool? inPrivate = null);

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

please update

@jjw24
Copy link
Member

jjw24 commented Jan 30, 2022

Hm if that's the case, I would doubt whether we should include a new public api because it is simply a helper function.

nothing about this suggests that you can pass an application Uri and it will be started as a process:

/// <summary>
/// Opens the url. The browser and mode used is based on what's configured in Flow's default browser settings.
/// </summary>
public void OpenUrl(string url, bool? inPrivate = null);

resolved.

@jjw24 jjw24 enabled auto-merge January 30, 2022 21:35
@jjw24 jjw24 merged commit af313eb into dev Jan 30, 2022
@jjw24 jjw24 deleted the websearchUri branch January 30, 2022 21:43
@jjw24
Copy link
Member

jjw24 commented Jan 30, 2022

@taooceros i dont think the logic prior to my change is correct. Can you give me an App URI example you were testing with.

@jjw24
Copy link
Member

jjw24 commented Jan 31, 2022

@taooceros sorry, my mistake. This is now fixed in PR1002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants