Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Device manager - add foundation for extended device info #9344

Merged
merged 31 commits into from
Oct 5, 2022

Conversation

kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented Oct 4, 2022

With matrix-org/matrix-js-sdk#2728

Mostly housekeeping:

  • Use ExtendedDevice type everywhere in new DM
  • Rename clientName/Version to appName/Version in types to avoid collision with UA parsed client
  • Add stubbed UA parsing function
  • Adjust tests for new required properties
  • Rename DeviceType component to DeviceTypeIcon to avoid collision

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

Kerry Archibald added 29 commits September 23, 2022 18:00
* add device selection that does nothing

* multi select and sign out of sessions

* test multiple selection

* fix type after rebase

* select all sessions
@kerryarchibald kerryarchibald added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Oct 4, 2022
Base automatically changed from psg-682/display-client-info to develop October 4, 2022 13:12
@kerryarchibald kerryarchibald marked this pull request as ready for review October 4, 2022 14:22
@kerryarchibald kerryarchibald requested a review from a team as a code owner October 4, 2022 14:22
Copy link
Contributor

@weeman1337 weeman1337 left a comment

Choose a reason for hiding this comment

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

Tested on the most recent deployment. The device manager explodes 💣 Can you have a look and then re-request review?

appBrand?: string,
): DeviceType => {
// this sucks
const safeBrowserName = appBrand && browser.name.indexOf(appBrand) > -1 ? undefined : browser.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

😆 Is there anything that sucks less? You could use includes instead of the indexOf check btw..

@kerryarchibald kerryarchibald force-pushed the psg-632/device-man-parse-ua branch from e6bbc36 to eef014a Compare October 4, 2022 15:50
@kerryarchibald
Copy link
Contributor Author

@weeman1337 sorry, way too many nested branches open. Something weird happened while I rebased this and I included more changes than I meant to 🙈
Changes should match the description now

Copy link
Contributor

@weeman1337 weeman1337 left a comment

Choose a reason for hiding this comment

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

Looks good now 👍

@kerryarchibald kerryarchibald enabled auto-merge (squash) October 5, 2022 10:17
@kerryarchibald kerryarchibald merged commit bd270b0 into develop Oct 5, 2022
@kerryarchibald kerryarchibald deleted the psg-632/device-man-parse-ua branch October 5, 2022 11:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants