Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Add new TabsUseCases for closing all sessions #1457

Closed
jonalmeida opened this issue Nov 23, 2018 · 5 comments
Closed

Add new TabsUseCases for closing all sessions #1457

jonalmeida opened this issue Nov 23, 2018 · 5 comments
Labels
🌟 feature New functionality and improvements good first issue Good for newcomers help wanted Extra attention is needed 💡idea Ideas for a new component, feature, change .. <tabs> Components: browser-tabstray, feature-tabs
Milestone

Comments

@jonalmeida
Copy link
Contributor

jonalmeida commented Nov 23, 2018

We need a use case that lets you close:

  • all
  • regular tabs only
  • private tabs only

┆Issue is synchronized with this Jira Task

@jonalmeida jonalmeida added help wanted Extra attention is needed good first issue Good for newcomers 🌟 feature New functionality and improvements <tabs> Components: browser-tabstray, feature-tabs 💡idea Ideas for a new component, feature, change .. labels Nov 23, 2018
@pocmo
Copy link
Contributor

pocmo commented Nov 23, 2018

FYI: In the context menu PR that I'm going to open now I renamed some of the use case classes inside TabsUseCases. There may be conflicts! :)

@chikka
Copy link
Contributor

chikka commented Nov 24, 2018

@jonalmeida @pocmo can i take it up?

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Nov 24, 2018

@chikka Definitely! Although you should wait for #1459 to get merged. If you're keen to work on this ASAP, then you can work off that branch.

Have a look at TabsUseCases.kt to get an idea of how to the new use cases.

@chikka
Copy link
Contributor

chikka commented Nov 26, 2018

@jonalmeida Thanks!. I have looked into it and I think we need to create two classes for it

class RemoveAllTabsUseCase internal constructor(
     private val sessionManager: SessionManager
 ) {
    
     fun invoke() {
         
     }
 }
  class RemoveTabsUseCase internal constructor(
       private val sessionManager: SessionManager
   ) {
      
//send privateOnly=true for closing only private tabs otherwise false to close all normal tabs
       fun invoke(privateOnly: Boolean = false) {
           
       }
   }

Please let me know your thoughts on it

@jonalmeida
Copy link
Contributor Author

@chikka looks great! Feel free to create a PR and we talk review it there as well.

Small nit:

fun invoke(privateOnly: Boolean = false) { }

I'd change the attribute name from privateOnly to private. It may imply that it would remove all tabs vs only private tabs.

chikka added a commit to chikka/android-components that referenced this issue Nov 27, 2018
chikka added a commit to chikka/android-components that referenced this issue Nov 27, 2018
chikka added a commit to chikka/android-components that referenced this issue Dec 13, 2018
chikka added a commit to chikka/android-components that referenced this issue Dec 13, 2018
@pocmo pocmo closed this as completed in ac33702 Dec 17, 2018
pocmo pushed a commit that referenced this issue Dec 17, 2018
@pocmo pocmo added this to the 0.35 🐠 milestone Dec 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🌟 feature New functionality and improvements good first issue Good for newcomers help wanted Extra attention is needed 💡idea Ideas for a new component, feature, change .. <tabs> Components: browser-tabstray, feature-tabs
Projects
None yet
Development

No branches or pull requests

3 participants