959184 - Use enum (EDIT, DISPLAY) instead of boolean to track BrowserToolbar mode

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Using an enum will make the code easier to read and it's more future-proof.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Use enum instead of boolean to track BrowserToolbar's mode (r=mcomella)

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8361092 [details] [diff] [review]
Use enum instead of boolean to track BrowserToolbar's mode (r=mcomella)

Review of attachment 8361092 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me with the var convention change.

::: mobile/android/base/toolbar/BrowserToolbar.java
@@ +144,5 @@
>  
>      final private BrowserApp mActivity;
>      private boolean mHasSoftMenuButton;
>  
> +    private UIMode mUiMode;

s/UiMode/UIMode/

We should be consistent, here and throughout the file.

uiMode (i.. as a local var) should probably stay like that.

@@ +848,5 @@
>              }
>          }
>      }
>  
> +    private void setUiMode(UIMode uiMode) {

nit: final

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #2)
> Comment on attachment 8361092 [details] [diff] [review]
> Use enum instead of boolean to track BrowserToolbar's mode (r=mcomella)
> 
> Review of attachment 8361092 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me with the var convention change.
> 
> ::: mobile/android/base/toolbar/BrowserToolbar.java
> @@ +144,5 @@
> >  
> >      final private BrowserApp mActivity;
> >      private boolean mHasSoftMenuButton;
> >  
> > +    private UIMode mUiMode;
> 
> s/UiMode/UIMode/
> 
> We should be consistent, here and throughout the file.
> 
> uiMode (i.. as a local var) should probably stay like that.

Done.

> @@ +848,5 @@
> >              }
> >          }
> >      }
> >  
> > +    private void setUiMode(UIMode uiMode) {
> 
> nit: final

final what?

Comment 4

[Lucas Rocha (:lucasr)]

Pushed with suggested changes (and assumed the final is for setUIMode's argument).

https://hg.mozilla.org/integration/fx-team/rev/b87878a98887

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b87878a98887

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Lucas Rocha (:lucasr) from comment #3)
> final what?

Method arg.

But yes, definitely a nit. :)