search

smaranjitghose / DocLense

An open-source document scanner!
https://doclense.vercel.app/
Creative Commons Zero v1.0 Universal
147 stars 130 forks source link

Fixed the pdf cards. #222

Closed abstrxtInfinity closed 3 years ago

abstrxtInfinity commented 3 years ago

Issue No. #221 #194

abstrxtInfinity commented 3 years ago

Can you please make all the colors associated with the PDF card dynamic? So they match with the theme. You can find such dynamic examples in the repository itself, for ex - line 101 in main_drawer.dart makes the color of Divider dynamic. (Some colors like the color of text are automatically and correctly picked up from the theme constants so they don't need to be defined here, rather the colors which are to be changed like color of the card, color of the icons and color of the shadow should be defined here)

I couldn't find any more places where it was required to change the theme dynamically. If you have any specific location. About what you said, on changing the theme yes, the text was taking the color from theme constants (white in dark mode), but the problem here is that the PDF card has white background in dark mode as well, hence the text was not visible. If the color is not to be hardcoded I will suggest changing the card background in dark mode.

Saransh-cpp commented 3 years ago

Can you please make all the colors associated with the PDF card dynamic? So they match with the theme. You can find such dynamic examples in the repository itself, for ex - line 101 in main_drawer.dart makes the color of Divider dynamic. (Some colors like the color of text are automatically and correctly picked up from the theme constants so they don't need to be defined here, rather the colors which are to be changed like color of the card, color of the icons and color of the shadow should be defined here)

I couldn't find any more places where it was required to change the theme dynamically. If you have any specific location. About what you said, on changing the theme yes, the text was taking the color from theme constants (white in dark mode), but the problem here is that the PDF card has white background in dark mode as well, hence the text was not visible. If the color is not to be hardcoded I will suggest changing the card background in dark mode.

Yes, I meant that maybe we should change the card's color in the dark mode to grey and keep the icons and text as per the theme constants, what do you suggest?

abstrxtInfinity commented 3 years ago

Can you please make all the colors associated with the PDF card dynamic? So they match with the theme. You can find such dynamic examples in the repository itself, for ex - line 101 in main_drawer.dart makes the color of Divider dynamic. (Some colors like the color of text are automatically and correctly picked up from the theme constants so they don't need to be defined here, rather the colors which are to be changed like color of the card, color of the icons and color of the shadow should be defined here)

I couldn't find any more places where it was required to change the theme dynamically. If you have any specific location. About what you said, on changing the theme yes, the text was taking the color from theme constants (white in dark mode), but the problem here is that the PDF card has white background in dark mode as well, hence the text was not visible. If the color is not to be hardcoded I will suggest changing the card background in dark mode.

Yes, I meant that maybe we should change the card's color in the dark mode to grey and keep the icons and text as per the theme constants, what do you suggest?

That sounds nice, I will implement it.

abstrxtInfinity commented 3 years ago

fixed

@Saransh-cpp will this do?

Saransh-cpp commented 3 years ago

@abstrxtInfinity the screenshot and the code looks good!