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

#1253 read action mode for sharing and deleting zim files on disk #1254

Merged
merged 4 commits into from
Jul 8, 2019

Conversation

macgills
Copy link
Contributor

@macgills macgills commented Jul 2, 2019

Fixes #1253

Changes: Create Redux-y-ish architecture to enable clean enabling of action mode.
There is still a lot to make better here

  • Side effects shouldn't return values
  • state rendering should be a side effect
  • this is only affecting ZimFileSelectFragment
  • state object should be a sealed class with distinct states
  • no unit tests

But it is about as much as I can do in my time remaining this week to unblock @Aditya-Sood from working on LocalFileTransfer

@macgills macgills requested review from abdulwd and mhutti1 July 2, 2019 15:40
mhutti1
mhutti1 previously approved these changes Jul 2, 2019
@macgills macgills requested a review from mhutti1 July 3, 2019 15:35
@kelson42 kelson42 changed the title #1253 readd action mode for sharing and deleting zim files on disk #1253 read action mode for sharing and deleting zim files on disk Jul 3, 2019
@soloturn
Copy link
Contributor

soloturn commented Jul 7, 2019

@macgills the commit comment does not help to understand the commit, violates the commit style guidelines here:
https://github.com/kiwix/kiwix-android/blob/develop/docs/commitstyle.md

@kelson42, still in emergency mode?

@Aditya-Sood
Copy link
Contributor

Aditya-Sood commented Jul 7, 2019

@soloturn hi, if you need the context: In the 'Get Content' activity, long-pressing a zim file item in the 'Library' doesn't show the contextual action bar in the kotlin port, but used to previously in the java code of master

That bar showed an option to share zims, crucial for the working of my deliverable feature/LocalFileTransfer

@soloturn
Copy link
Contributor

soloturn commented Jul 8, 2019

@macgills you mind ammending your commit comment with adityas message here, and squash your commits?

@Aditya-Sood
Copy link
Contributor

@soloturn not sure if my comment sums up everything done in this PR, all I know is that it was one of the things to be addressed

@codecov
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

Merging #1254 into develop will increase coverage by 0.11%.
The diff coverage is 19.42%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1254      +/-   ##
=============================================
+ Coverage      33.12%   33.23%   +0.11%     
  Complexity         1        1              
=============================================
  Files            165      173       +8     
  Lines           6416     6478      +62     
  Branches         699      704       +5     
=============================================
+ Hits            2125     2153      +28     
- Misses          4077     4104      +27     
- Partials         214      221       +7
Impacted Files Coverage Δ Complexity Δ
...ager/fileselect_view/adapter/BooksOnDiskAdapter.kt 75% <ø> (ø) 0 <0> (ø) ⬇️
...kiwix/kiwixmobile/extensions/ActivityExtensions.kt 0% <0%> (ø) 0 <0> (?)
...zim_manager/fileselect_view/effects/DeleteFiles.kt 0% <0%> (ø) 0 <0> (?)
.../zim_manager/fileselect_view/effects/SideEffect.kt 0% <0%> (ø) 0 <0> (?)
...ger/fileselect_view/adapter/BooksOnDiskListItem.kt 23.8% <0%> (-1.2%) 0 <0> (ø)
...mobile/zim_manager/library_view/LibraryFragment.kt 22.98% <0%> (ø) 0 <0> (ø) ⬇️
...mobile/zim_manager/fileselect_view/effects/None.kt 0% <0%> (ø) 0 <0> (?)
.../zim_manager/fileselect_view/effects/ShareFiles.kt 0% <0%> (ø) 0 <0> (?)
.../java/org/kiwix/kiwixmobile/main/MainActivity.java 28.1% <0%> (+0.03%) 0 <0> (ø) ⬇️
...le/zim_manager/fileselect_view/effects/OpenFile.kt 0% <0%> (ø) 0 <0> (?)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8953642...e1a346a. Read the comment docs.

@kelson42
Copy link
Collaborator

kelson42 commented Jul 8, 2019

@kelson42, still in emergency mode?

We are not in emergency mode anymore. But we have here different views about how to do things with git... and we have releases to achieve to do soon. My strategy for now is to stay with what we have (a few changes will be brought to commitstyle.md) and save our energy to a more in-depth discussion on all of this (see kiwix/overview#18) in fall.

@macgills macgills merged commit 58c99fd into develop Jul 8, 2019
@macgills macgills deleted the feature/macgills/#1253-action-mode branch July 8, 2019 14:02
@macgills
Copy link
Contributor Author

macgills commented Jul 8, 2019

Given the need to unblock Aditya, the upcoming commitstyle change and Isaac's approval I have merged this ticket

@Aditya-Sood
Copy link
Contributor

Thanks! I have created the PR: #1276

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.

Recreate Action Mode Context menu for sharing/deleting
5 participants