If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

PromptService should show lists in same way

NEW
Unassigned

Status

()

Firefox for Android
Theme and Visual Design
5 years ago
5 years ago

People

(Reporter: sriram, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
PromptService shows lists in two different ways. The context menu based lists are showed as a normal list on a white background. The <select> items follow the same behavior. However, the "Services.prompt.select" shows a spinner, with a listview floating over it.
(Reporter)

Comment 1

5 years ago
Created attachment 735996 [details] [diff] [review]
Part 1: Cleanup prompts

Eclipse made me cleanup these :(
Attachment #735996 - Flags: review?(wjohnston)
Comment on attachment 735996 [details] [diff] [review]
Part 1: Cleanup prompts

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

Love the cleanup stuff. Hate the switch statement stuff. Can you... do that different? I'd take a second patch for that if you want (but if so, I think we should leave the stuff that's there right now).

::: mobile/android/base/GeckoApp.java
@@ +1640,5 @@
>          if (SmsManager.getInstance() != null) {
>            SmsManager.getInstance().start();
>          }
>  
> +        mPromptService = new PromptService(this);

Nice. I've wanted to do this for a bit!

::: mobile/android/base/PromptService.java
@@ +53,5 @@
> +/**
> + * PromptService is used to show the input prompts for the webpages.
> + * This is a hook for the Gecko to show the inputs using Android's Native UI.
> + */
> +/*package*/ class PromptService implements OnClickListener, OnCancelListener, OnItemClickListener, GeckoEventResponder {

Split up these interfaces onto their own lines. Also, what's the *package* line for. I don't really like that.

@@ +81,5 @@
> +        TEXTBOX,
> +        PASSWORD,
> +        MENULIST,
> +        LABEL
> +    }

So... I don't want to do this this way. I'd rather split the input types up into subclasses and have them all implement an abstract class (instead of all these huge switch statements). You should be able to call:

public abstract static class PromptInput {
  public View getView();
  public Object getValue();
  static public PromptInput getInput(JSONObject obj) {
    if (obj.type.equals()) {
      return new CheckboxInput(obj);
    }
    ... etc.
  }
}


PromptInput input = PromptInput.getInput(jsonObject);
View v = input.getView();
String val = (String)input.getValue();
etc.

I'd rather do that than just shift around all these if/else statements into cases.

@@ +748,5 @@
>              }
>  
> +            view.setText(item.label);
> +            maybeUpdateCheckedState(position, item, view);
> +            maybeUpdateIcon(item, view);

Great!
Attachment #735996 - Flags: review?(wjohnston)
You need to log in before you can comment on or make changes to this bug.