Tabs to switch devices #183

xnkevinnguyen · 2020-01-28T00:24:08Z

Description:

Switch between microbit and cpx device

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Type of change

New feature (non-breaking change which adds functionality)

Limitations:

Please describe limitations of this PR

Testing:

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Test A
Test B

Checklist:

My code follows the style guidelines of this project
My code has been formatted with npm run format and passes the checks in npm run check
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

…derer Add basic test for device.tsx

… devices component Linting Remove unused ressource from device.tsx Update snapshot for device

src/view/container/device/Device.test.tsx

src/view/container/device/Device.tsx

Abstract tab in another component Abstract tab in another component Abstract tab in another component Load devices dynamically Abstract cpx Update snapshot

Add text in constants

src/view/App.tsx

src/view/constants.ts

sagarmanchanda

Amazing job 🥇 with the initial PR for microbit. Please make the suggested changes before checking it in.

src/view/App.spec.tsx

sagarmanchanda · 2020-01-31T18:35:22Z

src/view/App.spec.tsx

+import App from "./App";
+
+describe("App component should", () => {
+    it("render correctly", () => {


nit: "should render ...."

sagarmanchanda · 2020-01-31T18:36:17Z

src/view/App.tsx

                </main>
            </div>
        );
    }
+
+    handleDeviceChange = (item?: PivotItem) => {


nit: private

+1 for using arrow functions :D

sagarmanchanda · 2020-01-31T18:39:01Z

src/view/App.tsx

+}
+
+class App extends React.Component<{}, IState> {
+    state = {


I'd put this inside the constructor

this.state = initialState

Ok, declaring initial state makes it more readable

sagarmanchanda · 2020-01-31T18:40:50Z

src/view/components/cpx/Svg_utils.tsx

@@ -3,6 +3,8 @@

 // Adapted from : https://github.com/microsoft/pxt/blob/master/pxtsim/svg.ts

+/* tslint:disable */


What was the reason to add this?

That file is currently failing the tslint, but I will fix it

(unrelated to this pr)

sagarmanchanda · 2020-01-31T18:42:16Z

src/view/components/tab/Tab.tsx

+interface IProps {
+    handleTabClick: (item?: PivotItem) => void;
+}
+export class Tab extends React.Component<IProps, any> {


If it doesn't have a state, consider using React.FC

Also, instead of any, I'd use {} empty state to pass to React.Component

vandyliu · 2020-01-31T22:49:41Z

src/view/components/cpx/CpxImage.tsx

+            setupSwitch(this.props);
+            this.updateImage();
+        } else {
+            console.log("Cannot find svg");


Should we do more than just console.log()? Should we even include console.log(). Not very familiar with this situation to be honest

Removed the console log for now

…code-python-devicesimulator into users/t-xunguy/tabs

xnkevinnguyen requested review from abmahdy, adclements, iennae, isadorasophia, markAtMicrosoft, nasadigital, sagarmanchanda and smmatte as code owners January 28, 2020 00:24

xnkevinnguyen force-pushed the users/t-xunguy/tabs branch from 2300433 to 2313aff Compare January 28, 2020 00:36

xnkevinnguyen added 3 commits January 28, 2020 10:28

Add tabs with fabric library

b128324

Update react version to 16.9.0 to fix issue of typeerror for test ren…

f52f873

…derer Add basic test for device.tsx

Tabs are not rerendering components but only hiding/showing pertinent…

3037355

… devices component Linting Remove unused ressource from device.tsx Update snapshot for device

xnkevinnguyen force-pushed the users/t-xunguy/tabs branch from 2313aff to 3037355 Compare January 28, 2020 18:29

sagarmanchanda suggested changes Jan 28, 2020

View reviewed changes

xnkevinnguyen changed the title ~~WIP: Tabs to switch devices~~ Tabs to switch devices Jan 28, 2020

xnkevinnguyen force-pushed the users/t-xunguy/tabs branch from d97adc7 to 36ac97e Compare January 28, 2020 23:48

Fix pr with feedback

c1b5bee

Abstract tab in another component Abstract tab in another component Abstract tab in another component Load devices dynamically Abstract cpx Update snapshot

xnkevinnguyen force-pushed the users/t-xunguy/tabs branch from 36ac97e to c1b5bee Compare January 29, 2020 00:04

Add tests to components created and modified

c41db5a

Add text in constants

xnkevinnguyen force-pushed the users/t-xunguy/tabs branch from 6d689b9 to c41db5a Compare January 29, 2020 00:42

xnkevinnguyen requested a review from sagarmanchanda January 29, 2020 17:51

markAtMicrosoft suggested changes Jan 29, 2020

View reviewed changes

src/view/App.tsx Outdated Show resolved Hide resolved

src/view/constants.ts Outdated Show resolved Hide resolved

markAtMicrosoft self-requested a review January 29, 2020 21:12

Update micro:bit displayed word to match branding

f805013

markAtMicrosoft approved these changes Jan 29, 2020

View reviewed changes

xnkevinnguyen added 2 commits January 29, 2020 16:30

Refactor cpx image to a class component to have proper rerendering

3477a69

Update tests snapshots for ui changes

627711a

xnkevinnguyen mentioned this pull request Jan 30, 2020

micro:bit Image #184

Merged

13 tasks

sagarmanchanda approved these changes Jan 31, 2020

View reviewed changes

Merge branch 'dev' into users/t-xunguy/tabs

dd0bd12

vandyliu reviewed Jan 31, 2020

View reviewed changes

vandyliu approved these changes Jan 31, 2020

View reviewed changes

xnkevinnguyen added 3 commits February 3, 2020 08:46

Implement feedback from reviews

86f46d0

Merge branch 'users/t-xunguy/tabs' of https://github.com/microsoft/vs…

c63cc0a

…code-python-devicesimulator into users/t-xunguy/tabs

Remove outdated tests

b09b6c7

xnkevinnguyen merged commit 7d9bf29 into dev Feb 3, 2020

xnkevinnguyen deleted the users/t-xunguy/tabs branch February 3, 2020 22:54

		@@ -3,6 +3,8 @@

		// Adapted from : https://github.com/microsoft/pxt/blob/master/pxtsim/svg.ts

		/* tslint:disable */

Tabs to switch devices #183

Tabs to switch devices #183

Uh oh!

Conversation

xnkevinnguyen commented Jan 28, 2020 • edited by nasadigital Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Type of change

Limitations:

Testing:

Checklist:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sagarmanchanda left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xnkevinnguyen Jan 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xnkevinnguyen commented Jan 28, 2020 •

edited by nasadigital

Loading

sagarmanchanda left a comment •

edited

Loading

xnkevinnguyen Jan 31, 2020 •

edited

Loading