Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Dark theme #922

Closed
wants to merge 195 commits into from
Closed

[WIP] Dark theme #922

wants to merge 195 commits into from

Conversation

biohzrddd
Copy link
Contributor

@biohzrddd biohzrddd commented Apr 18, 2023

Ready to merge!

This is an overhaul of the way colors are used in Actual in order to add a dark theme. It also adds the possibility of future themes.

Please see the netlify preview here then give your opinion and rationale on any found issues either here on github or on our Discord feedback thread.

Dark
image

Light
image

@trafico-bot trafico-bot bot added the 🚧 WIP label Apr 18, 2023
@netlify
Copy link

netlify bot commented Apr 18, 2023

Deploy Preview for actualbudget failed.

Name Link
🔨 Latest commit 38eceb2
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/64bed987427152000878d4b3

@biohzrddd
Copy link
Contributor Author

Linking to #618

@j-f1 j-f1 linked an issue May 9, 2023 that may be closed by this pull request
@MatissJanis
Copy link
Member

👋 just checking in: are you planing to pick this up again? Any blockers or issues we could help with?

@biohzrddd biohzrddd closed this Jun 3, 2023
@biohzrddd biohzrddd force-pushed the dark-theme branch 2 times, most recently from d8135c6 to 400078d Compare June 3, 2023 17:46
@j-f1 j-f1 reopened this Jun 4, 2023
Comment on lines 110 to 109
let borderColor = selected ? colors.b8 : colors.border;
let borderColor = selected
? colors.tableBorderSelected
: colors.tableBorder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn’t appear to do anything? (the border color doesn’t change in either light or dark mode)

/>
)}
</View>
{!buttonsDisabled ? (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this button when there is no selection?

@@ -91,7 +96,7 @@ function Header({
}}
>
<CustomSelect
style={{ backgroundColor: 'white' }}
style={{ ...styles.smallText }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
style={{ ...styles.smallText }}
style={styles.smallText}

(or, alternately, apply this to the CustomSelect by default to fix the font sizing issue)

@@ -101,7 +106,7 @@ function Header({
/>
<View>to</View>
<CustomSelect
style={{ backgroundColor: 'white' }}
style={{ ...styles.smallText }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
style={{ ...styles.smallText }}
style={styles.smallText}

Comment on lines +565 to +566
<Text style={{ fontWeight: 600 }}>Upcoming dates</Text>
<Stack direction="column" spacing={1} style={{ marginTop: 10 }}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO these should go back to having a secondary text color so that they stand out less.

{state.isCustom && (
<Text
style={{
color: colors.b5,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this coloring, this doesn’t stand out as much anymore — can you make it a little more noticeable?

}}
onClick={() => onSwitchTransactions('matched')}
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here — why was this coloring removed?

default:
<Text>These transactions match this schedule:</Text>
<View style={{ flex: 1 }} />
<Text>Select transactions to link on save</Text>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

& here

transactions={state.transactions}
fields={['date', 'payee', 'amount']}
style={{
border: '1px solid ' + colors.border,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now the table area is missing a border when empty. Also somehow the table always has zero height so you can’t see any transactions.

@j-f1
Copy link
Contributor

j-f1 commented Jul 18, 2023

If you want me to help out with anything (merge conflicts, changes I’ve requested, etc) feel free to reach out! I don’t want you to burn out on this & also want to keep other PRs moving as possible.

@biohzrddd
Copy link
Contributor Author

Help needed to push this to completion.

@j-f1
Copy link
Contributor

j-f1 commented Jul 21, 2023

I’m starting work on breaking it up into pieces so it can be merged bit by bit — see #1367 and #1368.

Note to self: make sure you’re included in all the release note blurbs as an author.

@j-f1
Copy link
Contributor

j-f1 commented Jul 21, 2023

Progress so far:
Screenshot_2023-07-21 12 08 06
Screenshot_2023-07-21 12 35 48
Screenshot_2023-07-24 16 06 32

@biohzrddd biohzrddd mentioned this pull request Jul 30, 2023
@MatissJanis MatissJanis changed the title Dark theme [WIP] Dark theme Aug 14, 2023
@github-actions
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Sep 14, 2023
@github-actions
Copy link
Contributor

This PR was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Dark Mode
9 participants