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

Add search for token list, fixes #69 #86

Merged
merged 7 commits into from Nov 24, 2017

Conversation

Projects
None yet
2 participants
@friedger
Contributor

friedger commented Nov 6, 2017

Search field has been added to select token activity.

screenshot_1509898257

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 5, 2017

Owner

Do we need this here? Not sure when updateTokenList is used.

Do we need this here? Not sure when updateTokenList is used.

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 5, 2017

Owner

Better name would be TokenListAdapterCallback

Better name would be TokenListAdapterCallback

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 5, 2017

Owner

Filtering needs to be restored from some kind of state

Filtering needs to be restored from some kind of state

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 6, 2017

Contributor

Maybe move search field into action bar as suggested here
#84 (comment)

Search in the action bar feel more like one feature among other, not like being the main action. Not sure what you would like to have. However, the text field takes a lot of UI space.

Contributor

friedger commented Nov 6, 2017

Maybe move search field into action bar as suggested here
#84 (comment)

Search in the action bar feel more like one feature among other, not like being the main action. Not sure what you would like to have. However, the text field takes a lot of UI space.

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 6, 2017

Member

Yes - I think I would like it in the ActionBar a bit more - can also rewrite it to be like this if you don't want to

Member

ligi commented Nov 6, 2017

Yes - I think I would like it in the ActionBar a bit more - can also rewrite it to be like this if you don't want to

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 21, 2017

Member

Do you want to finish this - otherwise I would rework it so that it is in the ActionBar

Member

ligi commented Nov 21, 2017

Do you want to finish this - otherwise I would rework it so that it is in the ActionBar

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 24, 2017

Contributor

Working on it now!

Contributor

friedger commented Nov 24, 2017

Working on it now!

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 24, 2017

Contributor

The search value is not persisted when rotating but that should be handled by Android. We could open a new issue for that.

Contributor

friedger commented Nov 24, 2017

The search value is not persisted when rotating but that should be handled by Android. We could open a new issue for that.

@friedger

This comment has been minimized.

Show comment
Hide comment
@friedger

friedger Nov 24, 2017

Contributor

@ligi Feel free to review again :-)

Contributor

friedger commented Nov 24, 2017

@ligi Feel free to review again :-)

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 24, 2017

Member

YAY - lgtm!

just a minor detail:

screenshot_1511534409

the color is off in night mode - gets correct once you enter one char:

screenshot_1511534417

But I think I will still merge it - and open a follow-up ticket for this

Member

ligi commented Nov 24, 2017

YAY - lgtm!

just a minor detail:

screenshot_1511534409

the color is off in night mode - gets correct once you enter one char:

screenshot_1511534417

But I think I will still merge it - and open a follow-up ticket for this

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 24, 2017

Member

Found a fix for the color problem:

        <item name="colorControlHighlight">?android:textColorPrimary</item>
        <item name="android:textColorHint">?android:textColorPrimary</item>
        <item name="android:editTextColor">?android:textColorPrimary</item>

in styles.xml for wallethActionBarThemeOverlay

Member

ligi commented Nov 24, 2017

Found a fix for the color problem:

        <item name="colorControlHighlight">?android:textColorPrimary</item>
        <item name="android:textColorHint">?android:textColorPrimary</item>
        <item name="android:editTextColor">?android:textColorPrimary</item>

in styles.xml for wallethActionBarThemeOverlay

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 24, 2017

Member

When playing around with it a bit more I found a blocker though. It doesn't play nice with swipe-to delete. This still uses the old unsorted list with the index of the sorted one.

Member

ligi commented Nov 24, 2017

When playing around with it a bit more I found a blocker though. It doesn't play nice with swipe-to delete. This still uses the old unsorted list with the index of the sorted one.

@ligi

This comment has been minimized.

Show comment
Hide comment
@ligi

ligi Nov 24, 2017

Member

Will still merge it - this fix is quite trivial and will do it as a follow-up commit. Did you get the invitation to the organisation? Only then I can assign the issue to you which is a limitation for issuETH currently ..

Member

ligi commented Nov 24, 2017

Will still merge it - this fix is quite trivial and will do it as a follow-up commit. Did you get the invitation to the organisation? Only then I can assign the issue to you which is a limitation for issuETH currently ..

@ligi ligi merged commit 86bd33f into walleth:master Nov 24, 2017

5 checks passed

kontinuum/assemble result
Details
kontinuum/checkout checkout done
Details
kontinuum/lint result
Details
kontinuum/spoon result
Details
kontinuum/test result
Details

ligi added a commit that referenced this pull request Nov 24, 2017

ligi added a commit that referenced this pull request Nov 24, 2017

@friedger friedger deleted the friedger:69/search branch Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment