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

Delineate platform support of various deeplink types #1159

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

travissanderson
Copy link
Contributor

No description provided.

@travissanderson
Copy link
Contributor Author

@dshokouhi what do you think about breaking it down like this at the top of the URL Handler integration page?

:::info
If multiple servers are connected to an app, `fire_event` links will be handled using the first server in the list.
:::

## Send one shot location
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious what the use case for this is. Is it for if you don't share your location by default but want to once to trigger automations etc? Or to force a precise GPS location update? Maybe both are valid usages?

@travissanderson
Copy link
Contributor Author

Ah looks like https://companion.home-assistant.io/docs/integrations/universal-links does this same thing, but without the header.

@travissanderson
Copy link
Contributor Author

@dshokouhi @jpelgrom I think this is ready for merge, if it looks good to you

…istant into patch-2

# Conflicts:
#	docs/integrations/url-handler.md
@travissanderson
Copy link
Contributor Author

@dshokouhi @jpelgrom merge conflicts resolved

<thead>
<tr>
<th><strong>Deep Link Type</strong></th>
<th><img alt="Android" src="/assets/android.svg" /> Full & Minimal</th>
Copy link
Member

Choose a reason for hiding this comment

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

Specifying "Full & Minimal" doesn't add any value here, it can only confuse people. Why not simply: Android?

<td>✅</td>
</tr>
<tr>
<td>Call Service</td>
Copy link
Member

Choose a reason for hiding this comment

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

Use sentence case

Suggested change
<td>Call Service</td>
<td>Call service</td>

<td>✅</td>
</tr>
<tr>
<td>Fire Event</td>
Copy link
Member

Choose a reason for hiding this comment

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

Use sentence case

Suggested change
<td>Fire Event</td>
<td>Fire event</td>

<td>✅</td>
</tr>
<tr>
<td>Send Location</td>
Copy link
Member

Choose a reason for hiding this comment

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

Use sentence case and match heading

Suggested change
<td>Send Location</td>
<td>Send one shot location</td>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you have a preference of "Send one shot location" over "Send location"? I prefer the latter, I think because "one shot" is a bit redundant and maybe a little metaphorical, but happy to adopt whichever you prefer.

@@ -5,11 +5,41 @@ id: 'url-handler'

Home Assistant supports opening from other apps via URL.

Query parameters are passed as a dictionary in the call.
## Platform Compatibility
Copy link
Member

Choose a reason for hiding this comment

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

Use sentence case - the current page may not always match the standards but let's try to keep them in mind for new documentation.

<table className="core-table">
<thead>
<tr>
<th><strong>Deep Link Type</strong></th>
Copy link
Member

Choose a reason for hiding this comment

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

Consistency with your previous paragraph and use sentence case

Suggested change
<th><strong>Deep Link Type</strong></th>
<th><strong>Deeplink type</strong></th>

@jpelgrom
Copy link
Member

Please don't @ mention contributors just because you don't get a quick answer.

@travissanderson
Copy link
Contributor Author

Ah the mention was not because of impatience I just saw the other PR had gone in mentioning conflicts with this one, so I tagged you just to communicate that I resolved them. No offense was meant, but I'll not use mentions going forward.

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.

2 participants