marmistrz / QDict

This is an open source Android dictionary application which support 'stardict' format dictionaries.
https://namndev.github.io/2015-05-13-qdict/
29 stars 8 forks source link

Fixed the display issues on those pages #6

Closed EasyVector closed 3 years ago

EasyVector commented 3 years ago

Dear developer:

Hello! I am the creator of issue #5, and I did what I could and fixed those problems. I would genuinely appreciate it if you could kindly revise my code and leave me some advice. Thank you so much for your precious time!

Here are screenshots after my changes:

copy copy copy

and before:

copy copy copy

Thanks again! Blessings on your day! :)

marmistrz commented 3 years ago

Can you keep the indentation in your change? it makes it easier to review the changes and doesn't pollute the diffs.

EasyVector commented 3 years ago

OK! I would love to. :)

EasyVector commented 3 years ago

Hello! I have already revoked the indentation changes I made and the diffs in those files should be easier to see now. Thank you so much!

marmistrz commented 3 years ago

Thanks. I'll review your changes soon, hopefully today. I also want to set up GitHub Actions CI before we have this merged.

EasyVector commented 3 years ago

Alright, you make the call. :)

marmistrz commented 3 years ago

The changes involving recent words and "No dictionary available" are fine.

The new layout for the about page doesn't display well with different font size/DPI. Please see the screenshot below. The version should be display right next to the "Version:" label. Would nesting LinearLayouts work for you?

Screenshot_20210830-094214_QDict

EasyVector commented 3 years ago

OK, totally understand this. I would like to try LinearLayout later. And I will share the result with you when it is good for me. :)

marmistrz commented 3 years ago

OK, totally understand this. I would like to try LinearLayout later. And I will share the result with you when it is good for me. :)

sure, please update the PR when you have your changes tested

EasyVector commented 3 years ago

I use a LinearLayout to wrap the version strings. And it seems good now. Here are screenshots after my modifications:

copy copy
marmistrz commented 3 years ago

Btw. please rebase.

EasyVector commented 3 years ago

Hello! I tried to follow the request you demand, but I am not quite sure if I did it right or not. If I did not rebase properly, I give you my sincerest apologies and sorry for the inconvenience.

EasyVector commented 3 years ago

Hello, I just opened a new pull request #10, and sorry for the inconvenience.

EasyVector commented 3 years ago

I will close this pull request. :)