Add SetupWizard libs from AOSP #392
Conversation
|
It doesn't compile due to https://github.com/seedvault-app/setupcompat/blob/13644ae60092b3469c81dad57d58520916f7cb60/build.gradle#L19 Adding it there worked however. Also, setupdesign has it set to 28. @grote what's the best way to deal with this? |
|
Other build issues:
It has width and height set to |
|
Builds with: |
* We're forking just in case we need to make any changes Change-Id: Ic96fac9d3b1f41490e36e6f16fe831cb89d0977a
Change-Id: I0f6da69dec1f80f820a0789ca7eec90e55f06b90
Change-Id: I08711d0ae6efa0ad4423c8a3b8f3fdd34d7c5617
* seedvault-app/setupcompat#1 * seedvault-app/setupdesign#1 Change-Id: If9d0b020eb74d354db1c6257357095bbb7413c9a
* Based on the changes done to RestoreSet in commit "Seedvault: redesign SUW activities" I2effdf5f11c8d92899f328abf0027e7bfd1a57ba Change-Id: Ie81407ee79c16d6c40a352efa846a05eac1ada37
Change-Id: Ie8f7a4d45ea7f37c86d47002c3bd2046a92797f1
* We need to import setupcompat library for theming reasons so change this as well to be consistent with SetupWizard code Change-Id: I9ca64295a2a68ae6c691d877b7156c1c51235003
Change-Id: I2effdf5f11c8d92899f328abf0027e7bfd1a57ba
There was a problem hiding this comment.
I stopped half way through reviewing this, because I kept coming across the same issues. Also, since layout previews don't work, could I please get before and after screenshots?
To be honest, I am also a bit skeptical pulling in these two libraries as submodules is worth it for the relatively minor UI changes they bring while at the same time losing tooling support for the parts they do bring.
| topStub.inflate() | ||
| val header = topStub.inflate() | ||
| val suwLayout: GlifLayout = header.findViewById(R.id.setup_wizard_layout) | ||
| val icon: Drawable = suwLayout.getIcon() |
There was a problem hiding this comment.
If you open this in Android Studio, it tells you to use kotlin syntax instead: suwLayout.icon.
Same below in this file.
| val suwLayout: GlifLayout = header.findViewById(R.id.setup_wizard_layout) | ||
| val icon: Drawable = suwLayout.getIcon() | ||
| icon.setTintList(getColorAccent(requireContext())) | ||
| suwLayout.setIcon(icon) |
There was a problem hiding this comment.
If you open this in Android Studio, it tells you to use kotlin syntax instead: suwLayout.icon = icon.
Same below in this file.
| @@ -54,6 +62,11 @@ internal class RestoreFilesStartedFragment : Fragment() { | |||
| ): View { | |||
| val v: View = inflater.inflate(R.layout.fragment_restore_files_started, container, false) | |||
|
|
|||
| val suwLayout: GlifLayout = v.findViewById(R.id.setup_wizard_layout) | |||
| val icon: Drawable = suwLayout.getIcon() | |||
| icon.setTintList(getColorAccent(requireContext())) | |||
There was a problem hiding this comment.
Can't the GlifLayout handle icon tinting in a better way? Do we have to do these acrobatics here just to apply a tint? As this code change happens a lot, maybe worth looking into this? There's a sudIconTint attribute for example and a special theme. Also a IconMixin with a setIconTint() method which might help.
| style="?android:attr/progressBarStyleHorizontal" | ||
| android:layout_width="0dp" | ||
| android:layout_height="4dp" | ||
| android:layout_marginTop="16dp" |
There was a problem hiding this comment.
The progress was neatly anchored at the top before, now there's this empty space above. Is this intentional?
| android:text="@string/restore_choose_restore_set" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@+id/imageView" |
There was a problem hiding this comment.
imageView doesn't exist anymore in this layout, the constraint would need to be adapted.
| android:id="@+id/titleView" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_margin="16dp" |
There was a problem hiding this comment.
The margin is gone in the new version. Is this intentional?
| android:layout_marginTop="16dp" | ||
| android:layout_marginEnd="16dp" | ||
| app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" | ||
| app:layout_constraintBottom_toTopOf="@+id/skipView" |
There was a problem hiding this comment.
This bottom constraint got removed. Intentionally? I am skeptical that android:layout_height="0dp" will work like this.
Also, now the list is most likely overlaying (or underlaying) the skip view.
| android:textColor="?android:colorError" | ||
| android:textSize="18sp" | ||
| android:visibility="invisible" | ||
| app:layout_constraintBottom_toTopOf="@+id/skipView" |
There was a problem hiding this comment.
This is missing in the new layout most likely causing the view to jump to the top instead of being vertically centered.
| style="?android:progressBarStyleLarge" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| app:layout_constraintBottom_toTopOf="@+id/skipView" |
There was a problem hiding this comment.
This is missing in the new layout most likely causing the view to jump to the top instead of being vertically centered.
Will do. For testing I've been
We can re-consider your previous idea of just doing the same layout ourselves directly without these libs, and we can always copy paste certain stuff as needed (dimens / colors / etc) |
|
Before: After: The after is how the rest of the SetupWizard will look going forward (it's close to what Google's looks like), and thus Seedvault restore UI should match that. |
|
Closing this, we should implement the UI directly without the setuplibs to
It just has to look the same to make it seamless with the SetupWizard, doesn't need to use the same libs. The lib and Seedvault are both Apache2 so we can freely copy paste things as needed. |
TODO: Add submodule instructions to README or wiki