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

show bookmark item as toggled when page is first entered #1917

Merged
merged 1 commit into from Mar 22, 2022

Conversation

tahmid-23
Copy link
Contributor

before when a page was bookmarked and you clicked on the bookmark, in the top bar it would show the bookmark as deactivated, this loads the bookmark cache state when the menu is opened

@ahmedre
Copy link
Contributor

ahmedre commented Mar 12, 2022

salam - jazakumAllah khairan -
i am not able to reproduce this issue - how are you seeing it? I bookmarked something and then went to the bookmarks tab and clicked it, and the page properly shows as bookmarked.

also, one question - why not also check split like the existing code?
jazakumAllah khairan.

@tahmid-23
Copy link
Contributor Author

it looks like this only happens on the 0th page because android won't call onPageSelected for page 0. I thought this happened on all pages. this will just call the handler manually

also isDualPagesVisible will not only check that it's not split screen, but also that we're not showing translations in split screen. previous code was prone to an error there too.

@ahmedre
Copy link
Contributor

ahmedre commented Mar 22, 2022

onPageSelected should also be called for page 0, though not on the very first load - but I can't repro the bug you're trying to fix. the two things I've tried:

  • bookmark an ayah in sura Fatiha. hit back. click the ayah from the bookmarks list. long press ayah and it correctly shows bookmarked state.
  • bookmark page 1 and hit back. click page 1 from bookmarks. correctly shows bookmarked state.

@tahmid-23
Copy link
Contributor Author

1st page is surah al-ikhlas in the app code

@ahmedre
Copy link
Contributor

ahmedre commented Mar 22, 2022

jazakAllah khairan - I can repro and I am seeing what you're seeing where page 0 isn't set if it's the page that the person directly jumped to

Copy link
Contributor

@ahmedre ahmedre left a comment

Choose a reason for hiding this comment

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

nice catch, jazakumAllah khairan

@ahmedre ahmedre merged commit 81f4edd into quran:master Mar 22, 2022
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

2 participants