Tweak dark themes into a higher contrast

[donnywellson]

I have modified the dark themes. I made some changes to how they switch from light to dark. The changes aren't much. In the theme.dart file instead of using the case method to switch between light and dark themes, I created two static constructors that would be placed in the theme and darktheme parameter. I also moved all files I could find relating theme to a new folder called themes. It was the only way I could stop myself from being confused and making repeated mistakes. Because of that, I had to remove most unused imports and place new ones. So the pull request may say over a dozen files have changed but it's really not that many. I also created some custom themes and placed them in a folder under themes to make the theme.dart code easier to read. I changed the DesignereVariables.dark constructor's values into something that would give the app a higher contrast. If you are unsatisfied with how the dark theme of the app turned out I could change the values again

[chrisbobbe]

Thanks for the PR. Please make the following changes so that we can review it:

1. In the PR description, link the issue or previous conversation that prompted the PR. It's Tweak dark theme for slightly higher contrast #973, right?
2. Tidy up your PR branch into clear and coherent commits. If you need help with this, please ask in #git help in the development community.
3. Make sure all commits pass all tests (tools/check).
4. In the commit(s) that change the colors that are used, the commit message should link to the source of the new colors. Greg pointed to an updated Figma in Tweak dark theme for slightly higher contrast #973; I believe that means you don't have to choose any colors yourself.

[donnywellson]

Is the Issue now closed I can't find it anywhere

…

On Tue, Oct 8, 2024, 8:29 PM Chris Bobbe ***@***.***> wrote: Thanks for the PR. Please make the following changes so that we can review it: 1. In the PR description, link the issue or previous conversation that prompted the PR. It's #973 <#973>, right? 2. Tidy up your PR branch <https://zulip.readthedocs.io/en/latest/git/using.html#examine-and-tidy-your-commit-history> into clear and coherent commits <https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html>. If you need help with this, please ask in #git help in the development community <https://zulip.com/development-community/>. 3. Make sure all commits pass all tests (tools/check). 4. In the commit(s) that change the colors that are used, the commit message should link to the source of the new colors. Greg pointed to an updated Figma in #973 <#973>; I believe that means you don't have to choose any colors yourself. — Reply to this email directly, view it on GitHub <#991 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQZVVHL7QWBLGSU3AO3YET3Z2QW75AVCNFSM6AAAAABPSEIBSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBQGY3DANZWGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[donnywellson]

Tweak dark theme for slightly higher contrast #973
. In this pull request, I moved all files I could find that related to themes into a folder called themes to stop myself from being confused. I also edited the theme by making two static ThemeData returns. This seemed more understandable. I also created custom themes which I placed inside a folder called custom themes under the created themes folder. This was to make the code in themes more readable. I also created widget tests for the custom themes which I placed in the themes_test.dart file. I also add the get package dependency to the pubsec.yaml file to add in the custom them tests. I see that there is conflict in this pull request. I suspect it's from me adding the get dependency, moving files to the themes folder and removing imports beacus e of the moved files. Pleas review my pull request and inform me if there is something you'd like me to change

[gnprice]

You'll need to clean up the Git branch in order to make it possible to review this any further. See the resources Chris linked above.

[donnywellson]

In this final revision of my commit history, I didn't move any files in the themes folder, which I had done previously. Everything else remains the same. I created two static ThemeData returns in the theme.dart file because it makes the code easier to read and understand. The app.dart file now includes values for both the light and dark themes. I created custom themes and placed them in the themes folder. Additionally, I altered the values of DesignVariables.dark. I was advised to use Figma for this, but since I am still learning how to use Figma, I utilized the Visual Studio Code color editor instead. I also created tests for the custom themes to ensure that their values would switch as the app transitions from light to dark mode. If you are unsatisfied with the outcome of the dark theme mode, please let me know.

[gnprice]

This is a bit closer but the Git history still isn't clear enough to make this possible to review.

In particular there will need to be a commit that just changes the colors, and doesn't make other changes in the same commit.

[donnywellson]

The other changes I had made in the same commi that I can remember was when I technically didn't alter the code myself. Like the public.yaml and pubsec.lock files. And the windows files. I added some of them together because I technically didn't alter them myself. It did, when I ran flutter pub upgrade

…

On Sun, Oct 13, 2024, 9:24 PM Greg Price ***@***.***> wrote: This is a bit closer but the Git history still isn't clear enough to make this possible to review. In particular there will need to be a commit that just changes the colors, and doesn't make other changes in the same commit. — Reply to this email directly, view it on GitHub <#991 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQZVVHJ7CFJQO3Y6ZAMX6CTZ3LJIFAVCNFSM6AAAAABPSEIBSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBZGEYTGMBUGI> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[gnprice]

Git can tell you what the sequence of changes in your commits actually is. Then it enables you to revise the commits. Again, see the Git resources Chris linked above.

[donnywellson]

This is the same as my previous commit history except I have made a commit about altering the DesignVaruiables.dark(). I realize my error. The ThemeData returns and the DesgnVariables were in the same class, which caused me to make such a mistake. In one of my previous commit histories, I separated these two by placing them in different classes to prevent such a mistake, which I recommend doing, but in an attempt to resolve some conflicts between repositories, I abandoned the idea, for now. Please review my pull request and tell me the areas you are unsatisfied with,

[gnprice]

It looks like this PR is trying to make an original redesign of all our dark-theme colors. That isn't what this issue calls for.

See again what Chris said at #991 (comment) , the first comment above:

4. In the commit(s) that change the colors that are used, the commit message should link to the source of the new colors. Greg pointed to an updated Figma in Tweak dark theme for slightly higher contrast #973; I believe that means you don't have to choose any colors yourself.

[gnprice]

This also still doesn't meet our standards for a clean Git history. In particular it doesn't satisfy this point Chris mentioned above:

3. Make sure all commits pass all tests (tools/check).

because it's broken after the first commit (and in fact stays broken until near the end).

You need to invest more effort in reviewing your own work before asking others to review it for you. When we're just repeating the same points because they haven't been addressed, that's not a good use of our time or of yours.