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.

[donnywellson]

I do not understand what you mean by this. How else am I supposed to tweak the dark theme if I don't edit the DesignVariables.dark()?

[donnywellson]

This new commit history has been reorganized for improved review clarity. It adds a folder called themes to the project. In the themes folder, I placed a Dart file that returns the AppBarTheme. I altered the values of DesignVariables.dark, and I added widget tests to verify that the values of the AppBarTheme change when switching from light to dark mode.

[gnprice]

I do not understand what you mean by this. How else am I supposed to tweak the dark theme

The desired colors are specified from the issue. We've pointed you at that a couple of times already in this thread:
#991 (comment)
#991 (comment)

I'm closing this PR because it's not yet doing any of the things we need for this issue, and it doesn't look to be on track to become something we can merge.

---

Based on this thread, I think you don't yet have the skills to learn to contribute productively in the Zulip project. You're welcome to try again after at least 6 months.

I recommend you spend time working on other projects, and reading a lot of code, especially reading code in well-maintained codebases like Zulip or upstream Flutter (i.e., any of the main repositories at https://github.com/zulip or https://github.com/flutter). This advice from our contributing guide is applicable to a wide variety of codebases:

• Set up a development environment following the instructions in the project README.
• Start reading recent commits to see the code we’re writing. Use either a graphical Git viewer like gitk, or git log -p with the “secret” to reading its output.
• Pick some of the code that appears in those Git commits and that looks interesting. Use your IDE to visit that code and to navigate to related code, reading to see how it works and how the codebase is organized.