-
Notifications
You must be signed in to change notification settings - Fork 2
Experimental merge of designs #35
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
Conversation
aedwardg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good starting place, though ideally I'd like to keep PRs much smaller than this. This may just be a side-effect of not having a designer on the team though.
I think this is good to merge except there seems to be some git weirdness that happened when you were trying to merge in existing changes from main.
In the following screenshot, commits 1 and 2 shouldn't be there and 3 should be merging main into experimental-merge-of-designs.

I'm happy to help fix this if needed
| theme: { | ||
| colors: { | ||
| background: "#b3b3b3", | ||
| // background: "#4d4d4d", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get rid of this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just testing with a dark theme color. Something that can be looked at a later point in time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this CSS can be removed without changing the way the page looks. That can be done in a follow-up ticket though in order to move things forward.
| {/* </div>*/} | ||
| {/*</div>*/} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be removed
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&display=swap" | ||
| rel="stylesheet" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we're even using this font. I deleted this locally and nothing changed.
1636ef2 to
411a11c
Compare
What issue is this solving?
Closes #28
Description
Merged both design concepts.
Any helpful knowledge/context for the reviewer?
Feelings gif (optional)
Please make sure you've attempted to meet the following coding standards