quran / quran_android

a quran reading application for android
http://android.quran.com
GNU General Public License v3.0
2.01k stars 890 forks source link

Changing background color. #73

Closed mahmoudhossam closed 12 years ago

mahmoudhossam commented 13 years ago

I think the user should be able to choose the background color.

I personally find the default unreadable, but maybe that's just me.

hmaher commented 13 years ago

Yes.. we need to add that.. some users prefer other color themes for the background..

mahmoudhossam commented 13 years ago

Which activity is responsible for controlling this screen?

hmaher commented 13 years ago

we use a layout inflater for that like a custom view group.. checkout QuranPageFeeder.. may be @wnafee can give you more details...

mahmoudhossam commented 13 years ago

No, I'm not talking about loading content, I mean the activity that's active when reading pages.

QuranPageFeeder has a reference to PageViewQuranActivity, is that it?

hmaher commented 13 years ago

yes.. there are two subclasses for it.. check QuranViewActivity

mahmoudhossam commented 13 years ago

yeah, but PageViewQuranActivity seems to be the only one that handles UI, I think setting the background color should be done in this class.

I'm going to make a background color preference, this will be used in a setBackgroundColor method in PageViewQuranActivity which will be used inside of onCreate.

Should I go for it?

hmaher commented 13 years ago

ok.. let's try to check it out.. for our default set of images.. any way we are checking out how to support other types of mus7af (like mus7af el tagweed and el 7arameen) as many have requested them..

mahmoudhossam commented 13 years ago

I tried to do it, but it just won't change.

I tried making it transparent, but that only makes it white.

wnafee commented 13 years ago

I'm not sure what's not working for you exactly. But if you want to change the color of the page, look in the colors set in quran_page_layout.xml for the TextView and the HighlightingImageView.

If you want to let the user choose, then I suggest that it be done in the QuranPageFeeder (and similarly for the translation feeder) as they are being inflated and fed to the QuranPageCurlView.

I did a quick test on my side and it worked, so it should be fine inshaAllah.

mahmoudhossam commented 13 years ago

I know it works when you change the XML value, but that won't work at runtime.

I tried to do this, but it's not working anywhere.

Maybe you're doing it another way?

wnafee commented 13 years ago

As I commented above, you should do it in the QuranPageFeeder at the time the layout is being inflated for each page before they are fed to the QuranPageCurlView.

Look at the function called createPage(). This is where each page is being inflated. At that point, you can decide to set the color either immediately in this function or you can wait when the page is finally loaded from bitmap by doing it in PageDisplayer (which is called after PageRetriever thread is done).

Is that what you tried? (just fyi, I did that on my side at runtime and it's working fine)

mahmoudhossam commented 13 years ago

I tried to do it in createPage() and I got an InflationException for some reason.

Can't you at least write a gist containing the code you wrote?

wnafee commented 13 years ago

Here you go:

protected View createPage(int index) {
    View v = mInflater.inflate(mPageLayout, null);

    // Changing color on the fly
    TextView tv = (TextView)v.findViewById(R.id.txtPageNotFound);
    ImageView iv = (ImageView)v.findViewById(R.id.page_image);
    tv.setBackgroundColor(Color.RED); // random color set by user
    iv.setBackgroundColor(Color.GREEN); // another random color

    ScrollView sv = (ScrollView)v.findViewById(R.id.page_scroller);
    if (sv == null)
        v.setTag(new Boolean(false));
    else
        v.setTag(new Boolean(true));

    updateViewForUser(v, true, false);

    // Get page on different thread
    new PageRetriever(v, index).start();

    return v;
}
mahmoudhossam commented 13 years ago

that's going to set the background color for each individual view, I was trying to do something that would set the color for the whole activity.

I'll try this myself, if it works, I'll make a commit, thanks a lot.

wnafee commented 13 years ago

Naturally this should happen, there is nothing strange or wrong with setting the color for each view. think of the QuranPageFeeder like an Adapter. And think of createPage() as the equivalent of getView() for the Adapter. There's nothing strange about inflating the view there and setting what colors you want there.

Secondly, activities dont have colors. Views have colors. The link you posted gets the root view of the activity which is the ViewGroup that holds all of our other child views and set's the background color of that. (examples of possible root views are: linear layout, or frame layout or relative layout, scroll view etc)

Thirdly, even if you tried to set the color of the root view, it would not work for our case (or any other application that has a child view that fills the root view's viewport). It's like you have an empty wall and set it's color to red, then after that you put on top of it a fullscreen ImageView that's blue. The final thing that will show is the blue image view. You will not see the red for the root view because it's hidden behind the fullscreen child view. Setting color to root view does not trickle down to all children.

I hope that makes sense.

There is no other way than to set the color for every child view as they are being inflated... as one would normally do if he is working with and AdapterView with a customer Adapter. This is not a hack. This is normal

mahmoudhossam commented 13 years ago

Yeah, it does make sense, I knew something was blocking the color I was setting from showing, now I know what it is.

I'll add an option to change the background color to the options menu as well.

جزاك الله خيرا

wnafee commented 13 years ago

جزانا وإياكم