feat(browser): support Huawei browser #7453
Conversation
Reviewer's GuideSwitches browser/client detection to a new async, modular browser-tool implementation (including Huawei browser support) and aligns device naming from PC to Desktop across JS and .NET so table header fixing and client info work consistently on more browsers. Sequence diagram for the new browser-tool based getClientInfo flowsequenceDiagram
actor BrowserUser
participant ClientModule
participant BrowserTool
participant AjaxServer
BrowserUser->>ClientModule: getClientInfo(url)
ClientModule->>BrowserTool: getInfo([browser, system, device, language])
BrowserTool-->>ClientModule: browserInfo (browser, browserVersion, device, language, engine)
ClientModule->>ClientModule: build data (browser string, device, language, engine)
ClientModule->>AjaxServer: execute(url, invoke, method, data)
AjaxServer-->>ClientModule: response
ClientModule-->>BrowserUser: resolved client info request
Sequence diagram for async table reset and header fixing using browser-toolsequenceDiagram
actor User
participant TableComponent
participant TableJsModule as TableJs
participant BrowserTool
User->>TableComponent: trigger reset
TableComponent->>TableJsModule: reset(id)
TableJsModule->>TableJsModule: restore column order and DOM
TableJsModule->>TableJsModule: set scrollWidth
TableJsModule->>TableJsModule: await fixHeader(table)
TableJsModule->>BrowserTool: getInfo([device])
BrowserTool-->>TableJsModule: device info (device)
TableJsModule->>TableJsModule: if device != Desktop adjust right margin
TableJsModule-->>TableComponent: reset completed
Updated class diagram for browser/client detection and table modulesclassDiagram
class BrowserTool {
+parse(userAgent)
+getInfo(scopes)
+getFingerprint(scopes)
+isSupport(feature, value)
}
class ClientModule {
+ping(url, invoke, method)
+getClientInfo(url)
}
class TableModule {
+saveColumnOrder(options)
+reset(id)
+fixHeader(table)
}
class WebClientDeviceType {
<<enum>>
Desktop
Mobile
Tablet
}
ClientModule --> BrowserTool : uses getInfo
TableModule --> BrowserTool : uses getInfo
WebClientDeviceType ..> ClientModule : aligned device naming
WebClientDeviceType ..> TableModule : aligned device naming
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Renaming the C# enum member from
PCtoDesktopis a breaking change for any existing consumers (including persisted string values); consider adding a newDesktopmember and mapping from the old value instead of renaming in place. - The
resetandfixHeaderfunctions are nowasync, but callers ofresetin other modules may still treat it as synchronous; verify call sites and, if needed, return a resolved promise or introduce a new async API to avoid subtle timing regressions. - The large third-party
browser.min.mjsbundle is checked in as a single minified file, which is hard to review and maintain; consider adding the unminified source (or using a package manager build step) and keeping the minified output generated instead of hand-committed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Renaming the C# enum member from `PC` to `Desktop` is a breaking change for any existing consumers (including persisted string values); consider adding a new `Desktop` member and mapping from the old value instead of renaming in place.
- The `reset` and `fixHeader` functions are now `async`, but callers of `reset` in other modules may still treat it as synchronous; verify call sites and, if needed, return a resolved promise or introduce a new async API to avoid subtle timing regressions.
- The large third-party `browser.min.mjs` bundle is checked in as a single minified file, which is hard to review and maintain; consider adding the unminified source (or using a package manager build step) and keeping the minified output generated instead of hand-committed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7453 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32919 32919
Branches 4574 4574
=========================================
Hits 32919 32919
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR upgrades the browser detection functionality by replacing a custom browser.js implementation with the browser-tool v1.3.3 library to add support for Huawei browser detection and improve overall browser identification capabilities.
Key Changes
- Replaced custom browser detection with the browser-tool v1.3.3 library (browser.min.mjs)
- Updated API from synchronous
browser()to asynchronousbrowser.getInfo() - Changed device type from 'PC' to 'Desktop' for consistency with modern terminology
- Made corresponding async updates to Table.razor.js to accommodate the new API
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/wwwroot/modules/browser.min.mjs | New minified browser detection library (browser-tool v1.3.3) |
| src/BootstrapBlazor/wwwroot/modules/browser.js | Removed old custom browser detection implementation |
| src/BootstrapBlazor/wwwroot/modules/client.js | Updated to use new async browser.getInfo() API with proper parameters |
| src/BootstrapBlazor/Enums/WebClientDeviceType.cs | Renamed enum value from PC to Desktop (breaking change) |
| src/BootstrapBlazor/Components/Table/Table.razor.js | Made fixHeader function async and updated device type check from 'PC' to 'Desktop' |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump from 10.2.1-beta01 to 10.2.1-beta02 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export async function getClientInfo(url) { | ||
| const info = browser() | ||
| const info = await browser.getInfo(['browser', 'system', 'device', 'language']); |
There was a problem hiding this comment.
The API call has changed from synchronous to asynchronous. The old implementation called browser() synchronously, but the new implementation uses await browser.getInfo(['browser', 'system', 'device', 'language']). However, the function getClientInfo is already marked as async, so this change is compatible. Ensure that all callers of getClientInfo are handling the Promise correctly.
| /// | ||
| /// </summary> | ||
| PC, | ||
| Desktop, |
There was a problem hiding this comment.
The enum value has been renamed from PC to Desktop. This is a breaking change that could affect existing code that references WebClientDeviceType.PC. While the new name Desktop is more descriptive and aligns with modern terminology, this change should be clearly documented as a breaking change in the release notes, and consumers of this API will need to update their code.
| const b = window.browser() | ||
| if (b.device !== 'PC') { | ||
| const b = await browser.getInfo(); | ||
| if (b.device !== 'Desktop') { |
There was a problem hiding this comment.
The device type comparison has been changed from 'PC' to 'Desktop' to match the new browser detection library's return value. However, this hardcoded string should ideally match the enum value in WebClientDeviceType.cs. Since the C# enum now uses Desktop instead of PC, this change maintains consistency. Consider using a constant or enum value instead of a magic string to prevent future inconsistencies.
| margin = margin.replace('px', '') | ||
| const b = window.browser() | ||
| if (b.device !== 'PC') { | ||
| const b = await browser.getInfo(); |
There was a problem hiding this comment.
The call to browser.getInfo() is made without parameters, but in client.js the same method is called with parameters: browser.getInfo(['browser', 'system', 'device', 'language']). According to the browser-tool library API, both usages are valid - calling without parameters will fetch all available information. However, for consistency and performance, consider specifying only the needed properties. In this case, only the 'device' property is needed, so the call could be optimized to browser.getInfo(['device']).
| const b = await browser.getInfo(); | |
| const b = await browser.getInfo(['device']); |
Link issues
fixes #7452
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Integrate a new browser detection module and update client and table components to use its async API and revised device naming.
New Features:
Enhancements: