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

List editing #1104

Merged
merged 8 commits into from Mar 16, 2019

Conversation

Projects
None yet
3 participants
@charlag
Copy link
Collaborator

charlag commented Mar 5, 2019

  • Refactored ListsActivity & ListsViewModel
  • Added dialog for creating a new list and hooked it up with API
  • Added dialog for editing a list and partially hooked it up with API

#714

@connyduck

This comment has been minimized.

Copy link
Member

connyduck commented Mar 9, 2019

❤️
Do you want me to review this or to you still want to change things?

@charlag

This comment has been minimized.

Copy link
Collaborator Author

charlag commented Mar 9, 2019

I would like to finish errror handling, it's almost done.
ID you have an idea how do give AccountsInListDialogFragment round edges, it would be cool. I've tried some options but they didn't really work. If you have any other UI feedback it would also be cool. You can as also wait and do all at once later.

@connyduck
Copy link
Member

connyduck left a comment

Looks like you have to set a theme theme in the onCreate of the DialogFragment
setStyle(DialogFragment.STYLE_NORMAL, R.style.DialogFragmentStyle) and remove the background from the layout.

<style name="DialogFragmentStyle" parent="@style/ThemeOverlay.MaterialComponents.Dialog">
        <item name="dialogCornerRadius">8dp</item>
        <item name="android:backgroundTint">?attr/window_background</item>
</style>

its better but also not ideal 🤔

Show resolved Hide resolved app/src/main/res/layout/item_list.xml

@charlag charlag changed the title [WIP] List editing List editing Mar 10, 2019

@charlag

This comment has been minimized.

Copy link
Collaborator Author

charlag commented Mar 10, 2019

I've finished error handling and extracted strings (I hope everywhere). Your solution for dialog works really well, thanks!
Now it can be reviewed I believe 😉

Show resolved Hide resolved gradle.properties Outdated
@connyduck
Copy link
Member

connyduck left a comment

It would be awesome to get item change animations in the RecyclerView of the ListsActivity. I think you have to use a differ so individual changes are dispatched to the Adapter instead of a notifyDataSetChanged. (there is the convienience ListAdapter for this)

Cannot annotate unchanged lines, but in ListsActivity line 200+ it would be better to create the drawable in the right color instead of tinting it later.

charlag added some commits Mar 12, 2019

@charlag charlag force-pushed the list-edit branch from fe18b24 to 92e13f6 Mar 13, 2019

@charlag

This comment has been minimized.

Copy link
Collaborator Author

charlag commented Mar 13, 2019

I've applied all feedback I think except for the Drawable coloring suggestion because I'm not entirely sure what's your proposal and how to do it. Colorize it in advance in code? Extract it to the resource?

@connyduck

This comment has been minimized.

Copy link
Member

connyduck commented Mar 16, 2019

change animations <3

Colorize it in advance in code? Extract it to the resource?

I ll send a PR so you see what you mean.

@connyduck connyduck merged commit 520e0d6 into master Mar 16, 2019

1 check passed

ci/bitrise/a3e773c3c57a894c/pr Passed - Tusky
Details
@charlag

This comment has been minimized.

Copy link
Collaborator Author

charlag commented Mar 16, 2019

Thanks 💚

@charlag charlag deleted the list-edit branch Mar 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.