merge with master #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* added everything to the prompt-connected-start-testing-step and updated renderInitializer to allow us to trigger scanning from the android setup view * updated the prompt-connected-start-testing-step test files and snapshots; also made some styling changes to the dialog itself
We discussed this with our engineering team in our weekly "engineering discussion" yesterday to make sure everyone would understand what was going on with the new checks. Folks were excited to give it a try, and unanimously agreed that it would be a good idea to enable it. Thanks again, Justin!
* feat(android-setup): Add prompt-locate-adb state machine step * feat(android-setup): Add prompt-connect-to-device state machine step * feat(android-setup): Add detect-devices state machine step * feat(android-setup): Add new step configs to allAndroidSetupStateMachineConfigs
…oose-device dialog (#2858)
…2873) * feat(android-setup): Add detect-service state machine step * feat:(android-setup): Add prompt-install-service state machine step * feat(setup-android): Add installing-service state machine step * feat(setup-android): Add detect-permissions state machine step * feat(android-setup): Add prompt-grant-permissions state machine step * feat(android-setup): Add prompt-connected-start-testing state machine step * feat(android-setup): Add new step configs to allAndroidSetupStepConfigs
…2878) * Render nav links as buttons and change styles to work with button elements * render test links as buttons and requirement links as links
I notice the debug tools view got broken:
```
Uncaught TypeError: Cannot read property 'reflowUI' of undefined
at DebugToolsView (webpack-internal:///./src/debug-tools/components/debug-tools-view.tsx:8)
at renderWithHooks (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:14803)
at mountIndeterminateComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:17482)
at beginWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:18596)
at HTMLUnknownElement.callCallback (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:188)
at Object.invokeGuardedCallbackDev (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:237)
at invokeGuardedCallback (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:292)
at beginWork$1 (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:23203)
at performUnitOfWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22157)
at workLoopSync (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22130)
```
This is due to not checking the store data before using it on `DebugTools`.
Adding a guard clause to check if we actually have data before using it. If we don't, rendering a spinner.
**Note** I'm wondering if this guard clause is something we should put on the `withStoreSubscription` HOC instead on the wrapped components.
…alog (#2838) #### Description of changes Wires up the prompt-locate-adb step's Browse button the system open folder dialog. The dialog functionality can only be invoked from the main process, so round trips through the ipcRendererShim. I found it off-putting to have to update all the other steps' tests to add the new dep, so I updated them to use a common props builder to avoid that in the future (and simplify the other tests). Out of scope: hooking up the step's listeners to the state machine actions Animation of flow through dialog:  #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: Part 2 of 1724336 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [x] (UI changes only) Added screenshots/GIFs to description above - [x] (UI changes only) Verified usability with NVDA/JAWS
- Add functions to set the current device and available devices in the Android setup store - Modify setSelectionDevice action to take a DeviceInfo structure - Update detect-devices step to set available devices in store
#### Description of changes This PR, plus the scripts in [dbjorge/aiweb-strict-null-check-migration-tools](https://github.com/dbjorge/aiweb-strict-null-check-migration-tools), creates the initial infrastructure for an incremental migration to `strictNullChecks: true`, following the [same strategy that VS Code used](https://code.visualstudio.com/blogs/2019/05/23/strict-null). * Adds a secondary `tsconfig` file at `tsconfig.strictNullChecks.json` which is the same as our main/normal tsconfig file except that it enables `strictNullChecks` and only runs on the already-compliant parts of our code * Populates the initial `files` and `includes` of the `strictNullChecks` config with the subset of `/src/` that is already compliant (by using `autoAdd` from [aiweb-strict-null-check-migration-tools](https://github.com/dbjorge/aiweb-strict-null-check-migration-tools) * Adds a new `yarn null:check` command to compile against the new strictNullChecks config (note, an especially useful option during cleanup work is `yarn null:check --watch`) * Adds `yarn null:check` into `yarn fastpass` * Adds `yarn null:check` into PR/CI build The ongoing effort to incrementally bring the codebase to null safety would amount to iteratively using [aiweb-strict-null-check-migration-tools](https://github.com/dbjorge/aiweb-strict-null-check-migration-tools) to find the next candidate files, fixing up null safety issues there, and using that repo's `autoAdd` tool to bring in any transitively-fixed cases. #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: #2869 - [x] Ran `yarn fastpass` - [n/a] Added/updated relevant unit test(s) (and ran `yarn test`) - [n/a] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
…ead (#2893) * refactor: remove buttonflexcontainer classes and add css modules instead * remove not used scss
…the setup store (#2911) * feat(android-setup): Add code to set the app name from the device in the setup store The app name is required for the start testing screen, and it can't be set until after the permissions have been granted. It also can't be set in the onEnter function for the start testing step because the onEnter method is asynchronous, and the UI will therefore be visible possibly before the app name is set. * Fix linting errors * feat(android-setup): Modify configuringPortForwarding state machine step so that the application name is retrieved after port forwarding is successfully set up Port forwarding, not detect permissions, is when the device is set up and ready to get the app name. * Fix typo Co-authored-by: Dave Tryon <45672944+DaveTryon@users.noreply.github.com> * Update text for linguistic symmetry Co-authored-by: Dave Tryon <45672944+DaveTryon@users.noreply.github.com> Co-authored-by: Dave Tryon <45672944+DaveTryon@users.noreply.github.com>
* feat(android-setup): Add cancel action behavior to a few steps The expected behavior of the cancel action for each step is as follows: step | on cancel --- | --- detect-adb | no cancel / cancel = close prompt-locate-adb | no cancel / cancel = close prompt-connect-to-device | no cancel / cancel = close detect-devices | prompt-connect-to-device prompt-choose-device | no cancel (can rescan for devices or close) detect-service | prompt-choose-device prompt-install-service | prompt-choose-device installing-service | prompt-install-service prompt-install-failed | prompt-choose-device detect-permissions | prompt-choose-device prompt-grant-permissions | prompt-choose-device configuring-port-forwarding | prompt-choose-device prompt-configuring-port-forwarding-failed | prompt-choose-device prompt-connected-start-testing | no cancel (can rescan for devices or close) * Remove cancel actions for detect-* state machine steps These operations can't be canceled because we have no way to interrupt the calls -- we're awaiting promises. * Remove cancel action from configuring-port-forwarding state machine step There's no way to cancel the action of the step.
#### Description of changes This PR adds `scanPort` to `AndroidSetupStoreData`, sets up the `configuring-port-forwarding` step to set the new state, and configures the views that consume it to conditionally pick up the scan port from whichever of `AndroidSetupStore` or `DeviceStore` the `adbSetupView` feature flag says is canonical. This PR is setup work for both: * Removing the DeviceStore when we remove the feature flag * Enabling dynamic port selection Verified no functionality/UI change either with or without the adbSetupView flag #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [n/a] Addresses an existing issue: #0000 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
This PR uses the portfinder NPM package to have our service configurator dynamically choose a free host port to bind to, rather than unilaterally choosing the same port hardcoded into the service.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). SeeCONTRIBUTING.md.