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

Bottom toolbar hide text on help screen fixed #604

Closed
wants to merge 4 commits into from

Conversation

jb1998
Copy link

@jb1998 jb1998 commented Mar 8, 2018

Fixes #[555]

Changes: [I gave id to last cardview and I am then dynamically adding space when toolbar is enabled,the check is also made in onResume() of KiwixMobileActivity and if bottom toolbar is enabled then space is added accordingly]

Screenshots/GIF for the change: Below is the gif showing the changes.The text is now visible when we open the app and bottom toolbar is enabled as well as when we navigate from one screen in app to help screen.

@jb1998
Copy link
Author

jb1998 commented Mar 8, 2018

I see there are certain conflicts that are needed to be resolved.
I am resolving them.
Will get back soon :)

@brijeshshah13
Copy link
Contributor

@jb1998 What is the status of this PR?

@jb1998
Copy link
Author

jb1998 commented Mar 23, 2018

@brijeshshah13 I am doing a straight forward thing.
I am adding space at the bottom dynamically when toolbar is enabled.
But after resolving the conflicts I saw that after adding bottom space ,top space is also added.
So issue do get resolved ,the bottom text is not hidden but at top extra space comes up.
That was weird.
If you see the pull request I made I have written the straight forward code
params.setMargins(0, 0, 0, 56);
In this line I am adding space only for bottom (last parameter of function)

So to solve it I come up with the solution to remove the top space dynamically.
But then I thought we should know the cause of top space and do changes in that rather than just removing the top space.

I would like to make some changes in help.xml to see the cause.
If you got to know the cause by looking at code of this PR then do let me know I will make the desired changes :)

I will let you know in a day or two if I could make it from help.xml

Here is the screenshot displaying the weird space added at top

@brijeshshah13
Copy link
Contributor

@jb1998 Alright. Also, you might want to search for alternative solutions, just in case this approach doesn't work out.

@jb1998
Copy link
Author

jb1998 commented Mar 23, 2018

Yeah will definitely do that :)

@jb1998
Copy link
Author

jb1998 commented Mar 25, 2018

I am not able to build the project in android studio.
I made the code on my local system up-to-date with the current master branch but then I got error
"cannot resolve symbol LeakCanary"
Software:Android studio 3.0.1
I tried to find a solution , did whatever was written in the link
stackoverflow
github discussion
I wanted little help in it @brijeshshah13
Also , sorry for asking the doubt here as I could not find any other relevant place to ask my doubt to you.

@RohanBh
Copy link
Contributor

RohanBh commented Mar 25, 2018

@jb1998 An issue is open for this problem. See #664.

@brijeshshah13
Copy link
Contributor

@jb1998 Please try to Clean Project before you build the app again.

@jb1998
Copy link
Author

jb1998 commented Mar 26, 2018

I did Clean project and Invalidate caches but that did not work.

Finally changing build variant to debug worked. :)
Thanks @brijeshshah13 @RohanBh

@brijeshshah13
Copy link
Contributor

@jb1998 You mean kiwixDebug build variant?

@jb1998
Copy link
Author

jb1998 commented Mar 26, 2018

@brijeshshah13 Yes kiwixDebug build variant

fixed
@jb1998
Copy link
Author

jb1998 commented Mar 29, 2018

@brijeshshah13 I have fixed the issue
I gave id to last cardview and I am then dynamically adding space when toolbar is enabled,keeping "Keep toolbar visible" enabled

Please review it :)

@mhutti1 mhutti1 self-requested a review April 1, 2018 19:42
@abdulwd
Copy link
Contributor

abdulwd commented Aug 26, 2018

@jb1998 Thanks for the PR! Sorry, we are unable to merge this as it has been made redundant by the new HelpActivity (#803). Looking for future contributions from you. 🙂

@abdulwd abdulwd closed this Aug 26, 2018
@jb1998
Copy link
Author

jb1998 commented Aug 27, 2018

@abdulwd Thanks for informing :)

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.

4 participants