Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Jwen/download songlist delete #357

Merged
merged 38 commits into from
Jun 1, 2022
Merged

Conversation

jiabaow
Copy link
Collaborator

@jiabaow jiabaow commented May 28, 2022

change the delete song activity to download manager
show the downloaded songs in recycler view with a delete button

for now:
Screen Shot 2022-05-29 at 22 18 45

After clicking the delete button, the button disappears, but the view of the song list will only be updated once we re-enter the activity.

@jiabaow jiabaow self-assigned this May 28, 2022
private val songList: ArrayList<String>, private val listener: OnItemClickListener?
) : RecyclerView.Adapter<SongListAdapterForDelete.SongListForDeleteViewHolder>() {

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): SongListForDeleteViewHolder {
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 3 locations. Consider refactoring.


setContentView(R.layout.activity_download_manager)

Helper().setReturnToMainListener(
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 3 locations. Consider refactoring.

@jiabaow jiabaow marked this pull request as ready for review May 29, 2022 20:19
@jiabaow jiabaow linked an issue May 29, 2022 that may be closed by this pull request
@MaximeZmt MaximeZmt self-requested a review May 31, 2022 07:06
Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

I've tried the new feature on emulator.
Great Changes, but small remarks:

  • Could be great to have something more meaningful than "-" like maybe a small bin
  • Also when you click on the delete button, there is only the delete button that disappears but not the text. Could be great to remove the text too

@jiabaow
Copy link
Collaborator Author

jiabaow commented May 31, 2022

I've tried the new feature on emulator. Great Changes, but small remarks:

  • Could be great to have something more meaningful than "-" like maybe a small bin
  • Also when you click on the delete button, there is only the delete button that disappears but not the text. Could be great to remove the text too

Thanks for the remarks!
I will address the first one, and for the second one, I explained in the PR's description, that I will leave it like this now since I haven’t found a way to do that. If anyone has an idea how to achieve that, feel free to do it!

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

LGTM

@codeclimate
Copy link

codeclimate bot commented Jun 1, 2022

Code Climate has analyzed commit 2fd5e91 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 7

The test coverage on the diff in this pull request is 98.2% (80% is the threshold).

This pull request will bring the total coverage in the repository to 85.6% (0.0% change).

View more on Code Climate.

Copy link
Collaborator

@laurislopata laurislopata left a comment

Choose a reason for hiding this comment

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

LGTM

@MaximeZmt MaximeZmt merged commit 4afd8ee into main Jun 1, 2022
@MaximeZmt MaximeZmt deleted the jwen/download-songlist-delete branch June 1, 2022 10:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI Improvement of download/delete song list.
3 participants