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

Add Client Well-Known docs. #2078

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Add Client Well-Known docs. #2078

merged 3 commits into from
Sep 28, 2023

Conversation

pixlwave
Copy link
Member

Following on from element-hq/element-ios#7685, this PR adds documentation for all of the VectorWellKnown types handled in EI.

docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Johennes Johennes left a comment

Choose a reason for hiding this comment

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

If you have 2 minutes to add the web values from my comments, that'd be great. But otherwise looks amazing IMO. Thanks for doing this!

@pixlwave
Copy link
Member Author

@Johennes nice, updated with the feedback. Let's see if we can get feedback for Android too before merging.

Copy link
Member

@bmarty bmarty 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 adding this doc!

I have added comments about what is supported on EA.

EA also supports "m.tile_server"."map_style_url", which is not documented yet, if I am not mistaken.

Code (model) on EA: https://github.com/vector-im/element-android/blob/main/vector/src/main/java/im/vector/app/features/raw/wellknown/ElementWellKnown.kt#L23

docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
docs/client_well_known.md Outdated Show resolved Hide resolved
@pixlwave
Copy link
Member Author

pixlwave commented Sep 27, 2023

Super, thanks @bmarty, updated with your feedback. For the tile server, I think the documentation within the MSC should probably suffice? https://github.com/matrix-org/matrix-spec-proposals/blob/matthew/location/proposals/3488-location.md#well-known-configuration

@bmarty
Copy link
Member

bmarty commented Sep 28, 2023

For the tile server, I think the documentation within the MSC should probably suffice? https://github.com/matrix-org/matrix-spec-proposals/blob/matthew/location/proposals/3488-location.md#well-known-configuration

I was not sure if the idea of this doc was to centralize all the possible configurations for the .well-known file, that Element clients may understand. If this is the case, I would add a paragraph about map tiler, which can eventually contain a link to the MSC.

Element iOS does not seems to support the location configuration, that's why I though this was forgotten.

@pixlwave
Copy link
Member Author

Ah, EI does support the location configuration but its all defined in the SDK, so in the app it is handled in the configuration builder alongside the use of the VectorWellKnown models.

My original intent of this doc was to a place to capture Element specific configuration options that aren't documented anywhere outside of the code. But I can see where you're coming from, as EI also supports the device dehydration MSC in the well-known too. Maybe the best thing here would be to link to the respective parsers for each platform so its easy to find the implementations for the spec/msc documented support too?

We can do that in a follow-up though, for now I'm going to merge this PR.

@pixlwave pixlwave merged commit e0c6993 into develop Sep 28, 2023
@pixlwave pixlwave deleted the doug/well-known-docs branch September 28, 2023 09:12
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.

3 participants