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: back button in actionbar #1442

Merged
merged 2 commits into from Mar 28, 2019
Merged

fix: back button in actionbar #1442

merged 2 commits into from Mar 28, 2019

Conversation

anhanh11001
Copy link
Contributor

@anhanh11001 anhanh11001 commented Mar 26, 2019

Details:
Previously, when the error fetching message is displayed, the other menu items in the action bar are hided by setting setHasOptionMenu to false. This made the android.R.id.home menu item stop working.

So instead of setting the setHasOptionMenu to false, setting the menu item visibility to false solve problem

Fixes: #1436

Screenshots for the change:

Details:
Previously, when error fetching is displayed, other menu item in actionbar is hided by setting setActionMenu to false. This made the android.R.id.home menu item stop working.

So instead of setting the hasActionMenu to false, setting the menu item visibility to false solve problem

Fixes: #1436
Copy link
Member

@liveHarshit liveHarshit left a comment

Choose a reason for hiding this comment

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

LGTM

val menuItemSize = menuActionBar?.size() ?: 0
for (i in 0..(menuItemSize - 1)) {
menuActionBar?.getItem(i)?.isVisible = !show
}
Copy link
Member

Choose a reason for hiding this comment

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

And when we go back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but I don't really understand your question? @iamareebjamal

Copy link
Member

Choose a reason for hiding this comment

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

Does it reappear correctly when we go back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand, it will reappear correctly because:

  1. Menu Items visibility will only change on function showEventErrorScreen()
  2. When error fetching occurs, the error is observed and call that function showEventErrorScreen() to true -> menu items visibility is false -> disappear
  3. When error fetching stop occurring, the event detail is fetched and the event variable in the ViewModel is observed and call that function showEventErrorScreen() to false -> menu items visibility is true -> reappear

You can see it in here from line 84 to 104

       eventViewModel.event
            .nonNull()
            .observe(this, Observer {
                loadEvent(it)
                eventShare = it
                title = eventShare.name

                if (eventShare.favorite) {
                    setFavoriteIcon(R.drawable.ic_baseline_favorite_white)
                }

                if (runOnce) {
                    loadSocialLinksFragment()
                    loadSimilarEventsFragment()
                }
                runOnce = false

                Timber.d("Fetched events of id %d", safeArgs.eventId)
                showEventErrorScreen(false)
                setHasOptionsMenu(true)
            })

@iamareebjamal iamareebjamal merged commit 34298f1 into fossasia:development Mar 28, 2019
@anhanh11001 anhanh11001 deleted the 1436_back_button branch June 20, 2019 13:56
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.

None yet

4 participants