Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add multi-device-setup #2493

Merged
merged 69 commits into from
Mar 24, 2023
Merged

add multi-device-setup #2493

merged 69 commits into from
Mar 24, 2023

Conversation

r10s
Copy link
Member

@r10s r10s commented Mar 8, 2023

requires deltachat/deltachat-core-rust#4007 to be checked out
counterpart of deltachat/deltachat-ios#1822

  • wrapper
  • basic UI
  • show progress, hide QR code once scanned (this is also nice feedback)
  • keep transfer screens on (i think, the show screen is okay already, not sure about receiver)
  • add a hint not to leave the app / the transfer screens
  • add permanent notification for sender
  • add permanent notification for receiver
  • create a dedicated activity also for the receiver
  • use "X" in the upper left corner instead of "<-" to make visually clear, things are aborted when leaving
  • protect sending backups by system credentials (as for export-backup or export-keys)
  • add a hint that QR codes copied to clipboard are invalidated on leaving the activity (in the "Abort transfer?" alert, only in case a code was copied)
  • improve wording and layout, make things translatable
  • add a real progress bar
  • fix activity leave bug -> moved to leaving setup-second-device activity leads to weird app state  #2507

sender:

 

receiver:

 

@r10s r10s force-pushed the multi-device-qr branch 4 times, most recently from 368f21e to 5ecc8a5 Compare March 9, 2023 20:48
@r10s r10s force-pushed the multi-device-qr branch 2 times, most recently from 32b2f45 to 5b55422 Compare March 9, 2023 22:31
@r10s r10s mentioned this pull request Mar 11, 2023
@r10s r10s force-pushed the multi-device-qr branch 7 times, most recently from 3a3c111 to 2ef7382 Compare March 14, 2023 20:22
@r10s r10s force-pushed the multi-device-qr branch 7 times, most recently from fedf317 to 1270e9a Compare March 19, 2023 10:37
@r10s
Copy link
Member Author

r10s commented Mar 22, 2023

hm, maybe better to put 1-3 directly below each other

@adbenitez
Copy link
Member

hm, maybe better to put 1-3 directly below each other

was thinking exactly that while looking the picture

@deltachat deltachat deleted a comment from github-actions bot Mar 22, 2023
@deltachat deltachat deleted a comment from github-actions bot Mar 22, 2023
@deltachat deltachat deleted a comment from github-actions bot Mar 22, 2023
this seems slightly catchier as "Add Another Device"
and has less "A" esp. in "Add as Another Device" :)

also at least translation to german seems nicer ("Zweitgerät"),
most ppl will use max. two devices, but even if more,
that should still be fine.

this was also the first intuition also by other devs in their mockups,
so we'll give it a try.
let's give that a try - "Account" is also widely used,
so that seems to make some sense.
even if not 100% fitting,
it seems better than introducing the new term "Collection".
Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

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

I didn't finish reviewing yet, but here come the first few comments - the comment on strings.xml is the only really discussion-worthy one, the others are just things I noticed but that would be fine as-is.

jni/dc_wrapper.c Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
res/layout/backup_provider_fragment.xml Outdated Show resolved Hide resolved
res/values/strings.xml Outdated Show resolved Hide resolved
res/values/strings.xml Outdated Show resolved Hide resolved
@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

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

I finished my review - apart from discussing out the wording, this LGTM 🎉

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s r10s merged commit 3cede34 into master Mar 24, 2023
@r10s r10s deleted the multi-device-qr branch March 24, 2023 14:22
@r10s
Copy link
Member Author

r10s commented Mar 24, 2023

merged that in already, i'll fix the open known bug in a subsequent pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants