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

Expose userAgent as get only property #75

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

jpsphaxer
Copy link
Contributor

@jpsphaxer jpsphaxer commented Jan 24, 2025

Expose User Agent String in HotwireConfig

Motivation

Previously in TurboNavigator, we could access the user agent string through TurboConfig. This functionality was lost in the migration to HotwireNative. Applications often need to maintain consistent user agent strings across different components (HTTP clients, web views, etc.) to ensure proper tracking and identification.

Changes

  • Added read-only userAgent property to HotwireConfig
  • Added test to verify property functionality

Testing

Added HotwireConfigTests with coverage for user agent string generation.

No breaking changes were introduced.

@joemasilotti
Copy link
Member

Thanks for this @jpsphaxer! I'm all for making this property get-able, too.

My only concern is the name. userAgent might imply the full user agent of the application, with WebKit and all that. But that isn't the case - we actually can't read that from WKWebView at all!

I'm thinking we might want to rename the getter to something that makes it obvious this is only the part provided by Hotwire Native and not the system. I'm open to ideas on what a good name might be.

@jpsphaxer
Copy link
Contributor Author

Thanks for this @jpsphaxer! I'm all for making this property get-able, too.

My only concern is the name. userAgent might imply the full user agent of the application, with WebKit and all that. But that isn't the case - we actually can't read that from WKWebView at all!

I'm thinking we might want to rename the getter to something that makes it obvious this is only the part provided by Hotwire Native and not the system. I'm open to ideas on what a good name might be.

what do you think of hotwireUserAgentComponent ? - naming is never my strong suit 😅

@joemasilotti
Copy link
Member

Maybe even just userAgentComponent since we are already in the Hotwire Native space.

What do you think @svara?

@jpsphaxer
Copy link
Contributor Author

Maybe even just userAgentComponent since we are already in the Hotwire Native space.

What do you think @svara?

yeah good call - I like that. I'll update the PR once we have consensus on the naming

@jpsphaxer
Copy link
Contributor Author

@joemasilotti - went ahead and updated the naming to userAgentComponent for the time being unless we decide to change the naming.

@svara
Copy link
Contributor

svara commented Jan 28, 2025

Maybe even just userAgentComponent since we are already in the Hotwire Native space.

What do you think @svara?

Sorry for late reply. userAgentComponent sounds good to me 👍🏻

Copy link
Contributor

@svara svara left a comment

Choose a reason for hiding this comment

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

Nicely done @jpsphaxer 👏🏻

@svara
Copy link
Contributor

svara commented Jan 29, 2025

FYI, we've decided to rename userAgentComponent to userAgent to align the API available in the Android library hotwired/hotwire-native-android#94.

@svara svara merged commit 4101cba into hotwired:main Jan 29, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants