add import and export capabilities for devices and suites by violetadev · Pull Request #1268 · responsively-org/responsively-app

violetadev · 2024-07-28T21:32:45Z

✨ Pull Request

📓 Referenced Issue

Fixes: #1258

ℹ️ About the PR

This PR reintroduces import/export capabilities in order to users to back up their settings and import them back.

🖼️ Testing Scenarios / Screenshots

respons.mp4

CLAassistant · 2024-07-28T21:32:53Z

All committers have signed the CLA.

manojVivek

You pulled it off so nicely!! 👏🏼
Just a couple of minor things, looks great otherwise!

manojVivek · 2024-07-29T03:40:42Z

desktop-app/src/renderer/store/features/device-manager/index.ts

+      window.electron.store.set('deviceManager.previewSuites', []);
+      state.suites = action.payload;
+      state.activeSuite = action.payload[0].id;
+      window.electron.store.set('deviceManager.previewSuites', action.payload);


~~This looks like it will override the existing suites as well, wouldn't that result in a loss of data if the user already had any custom suites?~~

I just noticed that there was a warning message indicating this, should we try to append it to the existing suites instead of replacing it?

sure, should we add also a button to reset the whole thing so they can go to the initial clean state as well?

manojVivek · 2024-07-29T03:42:20Z

.all-contributorsrc

      "contributions": [
        "code"
      ]
+    },


You can remove this, as it will be autopopulated by allContributors bot that updates the README as well.

manojVivek · 2024-07-29T03:43:23Z

desktop-app/src/renderer/components/FileUploader/FileUploader.test.tsx

+const mockHandleUpload = jest.fn();
+const mockResetUploadedFile = jest.fn();
+
+describe('FileUploader', () => {


Really appreciate the unit tests! 🫡

I wanted to add more on the ManageSuitesTool but I am having issues with the electron store, I can't seem to mock it. Do you have any idea how to mock it properly?

This is the error:

FAIL src/renderer/components/DeviceManager/PreviewSuites/ManageSuitesTool/ManageSuitesTool.test.tsx ● Test suite failed to run TypeError: Cannot read properties of undefined (reading 'store') 4 | import { sanitizeSuites } from './utils'; 5 | > 6 | const activeDeviceIds: string[] = window.electron.store.get( | ^ 7 | 'deviceManager.activeDevices' 8 | ); 9 |

Maybe we need to add a mock electron object to the window before the test runs?

Something like this:

electron-mock.ts

const store = {}; window.electron = { store: { get: (key: string) => { return store[key]; }, set: (key: string, val: any) => { store[key] = val; } } };

And then import this file at the top of the test file?

(haven't thought this through, just a quick suggestion)

I tried, I think we need to add a setup file for jest in the root. I will investigate further in a different pr

manojVivek · 2024-07-29T03:56:18Z

Also on the UI layout, shall we try the following placement for the icons:

And remove the dividers so that it will indicate that both suites and devices will be exported/imported.

manojVivek · 2024-07-29T04:04:02Z

...pp/src/renderer/components/DeviceManager/PreviewSuites/ManageSuitesTool/ManageSuitesTool.tsx

+          onClick={() => setOpen(true)}
+        >
+          <Icon
+            icon="mdi:folder-upload"


I'm leaning towards uil:import and uil:export for the icons?
As the current ones look more like a folder import/export to me. Wdyt?

violetadev · 2024-07-29T07:53:23Z

Also on the UI layout, shall we try the following placement for the icons:

And remove the dividers so that it will indicate that both suites and devices will be exported/imported.

yes, this was my first approach, but there are a few issues with this:

The import / export refers to importing not only suites, but also custom devices created by the user, and that title refers to the suites.
If I add it there, then I have to contaminate the PreviewSuites component with the setCustomDevices (which is unrelated) and then do prop drilling to the new component because we don't have a disptach action for it, so I need to pass it as prop.

I suggest instead we do something like this to improve the user experience (I did this over the screenshot and it's not very polished but so you get the idea):

Adjust font sizes and add accordion for each of the sections: suites, default devices and custom devices
Move the title from the top to under the x space, so we can align any settings CTAs we have that are global to this screen

if you are ok with this I can implement it in this same PR or open a new one following up (which one do you prefer?)

violetadev · 2024-07-29T21:05:39Z

Now instead of replacing, it merges whatever was there with the new.

replace.mp4

Also implements accordion and rearranges UI

newui.mp4

manojVivek · 2024-07-30T04:30:36Z

Also on the UI layout, shall we try the following placement for the icons:
And remove the dividers so that it will indicate that both suites and devices will be exported/imported.

yes, this was my first approach, but there are a few issues with this:

The import / export refers to importing not only suites, but also custom devices created by the user, and that title refers to the suites.

If I add it there, then I have to contaminate the PreviewSuites component with the setCustomDevices (which is unrelated) and then do prop drilling to the new component because we don't have a disptach action for it, so I need to pass it as prop.

I suggest instead we do something like this to improve the user experience (I did this over the screenshot and it's not very polished but so you get the idea):

Adjust font sizes and add accordion for each of the sections: suites, default devices and custom devices

Move the title from the top to under the x space, so we can align any settings CTAs we have that are global to this screen

if you are ok with this I can implement it in this same PR or open a new one following up (which one do you prefer?)

Yes, I like the described approach. Let's go with that.

remove changes

manojVivek · 2024-07-31T11:00:57Z

Merging this in now. You pulled it off so great for your first commit to the repo! @violetadev 👏🏼

Thank you so much! Looking forward to more like this in the future.

manojVivek · 2024-07-31T11:01:23Z

@all-contributors Please add @violetadev for code.

allcontributors · 2024-07-31T11:01:33Z

@manojVivek

I've put up a pull request to add @violetadev! 🎉

violetadev added 5 commits July 27, 2024 13:16

feature

744cdff

import

c177880

clean up

1cea3e8

add contributor

a40e65b

cleanup

df626db

violetadev marked this pull request as ready for review July 28, 2024 21:42

manojVivek requested changes Jul 29, 2024

View reviewed changes

manojVivek reviewed Jul 29, 2024

View reviewed changes

violetadev added 2 commits July 29, 2024 22:04

add btns

5a84916

fix

7547560

Update .all-contributorsrc

41031ea

remove changes

violetadev requested a review from manojVivek July 30, 2024 10:53

manojVivek merged commit 3a00333 into responsively-org:main Jul 31, 2024

allcontributors bot mentioned this pull request Jul 31, 2024

add violetadev as a contributor for code #1269

Merged

Uh oh!

Conversation

violetadev commented Jul 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Pull Request

📓 Referenced Issue

ℹ️ About the PR

🖼️ Testing Scenarios / Screenshots

Uh oh!

CLAassistant commented Jul 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manojVivek left a comment

Choose a reason for hiding this comment

Uh oh!

manojVivek Jul 29, 2024 • 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!

violetadev Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

manojVivek Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

manojVivek Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

violetadev Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

manojVivek Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

violetadev Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

manojVivek commented Jul 29, 2024

Uh oh!

manojVivek Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

violetadev commented Jul 29, 2024

Uh oh!

violetadev commented Jul 29, 2024

Uh oh!

manojVivek commented Jul 30, 2024

Uh oh!

manojVivek commented Jul 31, 2024

Uh oh!

manojVivek commented Jul 31, 2024

Uh oh!

allcontributors bot commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

violetadev commented Jul 28, 2024 •

edited

Loading

CLAassistant commented Jul 28, 2024 •

edited

Loading

manojVivek Jul 29, 2024 •

edited

Loading