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

fix: added tag doesn't reflect in adapter #8763

Merged
merged 2 commits into from
May 3, 2021

Conversation

TarekkMA
Copy link
Contributor

@TarekkMA TarekkMA commented May 3, 2021

Pull Request template

Purpose / Description

I have reverted to the old way of handling mFilteredList.

Fixes

Fixes #8762

Approach

mFilteredList will be only set when there is a search filter active, otherwise, it's null and the adapter will use the original full list.

How Has This Been Tested?

Emulator

Checklist

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@mikehardy
Copy link
Member

Is there a way to do a test here? Seems like robolectric should be able to drive the activity enough that you can check it and verify the regression + the fix? I'll admit, these kinds of tests where it deals with a few UI elements at once are a bit involved, but the fragment stuff is so new it would be nice to have more testing exercising it

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label May 3, 2021
@mikehardy mikehardy added this to the 2.15 release milestone May 3, 2021
@TarekkMA
Copy link
Contributor Author

TarekkMA commented May 3, 2021

Is there a way to do a test here? Seems like robolectric should be able to drive the activity enough that you can check it and verify the regression + the fix? I'll admit, these kinds of tests where it deals with a few UI elements at once are a bit involved, but the fragment stuff is so new it would be nice to have more testing exercising it

I will try, the issue I think is the involvement of RecyclerView. will see if I can use robolectric to deal with it

@TarekkMA
Copy link
Contributor Author

TarekkMA commented May 3, 2021

@mikehardy done.

I had to add these lines to make sure the adapter is updated to the new data.

            recycler.measure(0, 0);
            recycler.layout(0, 0, 100, 1000);

@TarekkMA TarekkMA force-pushed the tags-adapter-fix branch from dbe54f8 to 2bf512f Compare May 3, 2021 19:37
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, lgtm

@mikehardy mikehardy removed the Needs Author Reply Waiting for a reply from the original author label May 3, 2021
@mikehardy mikehardy merged commit 388b8e7 into ankidroid:master May 3, 2021
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.

[Bug] Regression added tag in TagsDialog don't appear in RecyclerView
2 participants