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

ui+settings: update theme colors + border colors #404

Merged
merged 1 commit into from
Aug 8, 2022
Merged

Conversation

dstrukt
Copy link
Contributor

@dstrukt dstrukt commented Aug 4, 2022

Apologies, should have probably done this in the last PR, but didn't fully know what it would entail, so didn't want to try to do everything in a single PR.

Just addresses history and settings to button up the basic things for the time being.

ui+settings: update theme colors + update border color on settings and history

Screenshots:
Screen Shot 2022-08-04 at 4 19 29 PM
Screen Shot 2022-08-04 at 4 19 32 PM
Screen Shot 2022-08-04 at 4 20 02 PM

@dstrukt dstrukt requested a review from jamaljsr August 4, 2022 23:19
jamaljsr
jamaljsr previously approved these changes Aug 5, 2022
Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK LGTM 👍

Can you get the unit tests passing? Then we can merge this.

@dstrukt
Copy link
Contributor Author

dstrukt commented Aug 5, 2022

Nice, and will do, but I wasn't quite sure why the unit tests were failing here .. trying to improve my ability to read these debug logs.

@jamaljsr
Copy link
Member

jamaljsr commented Aug 8, 2022

You can click on the Details link next to the failing check below. The logs show the 2 failing tests. It looks like these tests are checking the colors of the DOM elements. You just need to update the values in the test to match the new colors.

When working locally on your machine, you can run yarn test to watch for changes and run tests every time you save a file. You can also run yarn test:ci which will run all tests. This is what the GitHub workflow is running.

Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK LGTM 👍

@jamaljsr jamaljsr merged commit 4118bbd into master Aug 8, 2022
@jamaljsr jamaljsr deleted the settings-update branch August 8, 2022 19:37
dstrukt added a commit that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants