-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feat/rererereredesign2 #105
Conversation
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.
Great start! Couple points of feedback.
Spacing looks a bit cramped below. Would this be something that we could adjust? Also, the corresponding height and width for images seems a bit cramped as well.
I noticed you opted to use the styling of the events page. Is this there a reason for this? Could I suggest keeping styling separate since I assume similar to the above problem, there may be classes or styles that differ especially spacing wise between the two pages.
It is more advantageous to uniformize styles in code so we can avoid visual inconsistencies. The problem with image sizing is due to a discrepancy in how events.tsx and archive.tsx size images of arbitrary dimensions-- I will fix that. The archive popup is essentially the event popup with an extra section, so I can just adjust the spacing between old/new sections if the original spacing of the event popup is fine. I don't think either of these problems warrants separate styling. |
…aacm.com into feat/rererereredesign2
So I will use |
Also, quick suggestion. Are there better SVG assets that are more similar in style to the pure white assets we use on the home page? This may make things fit more in style. |
If we want to drastically change archive/event cards in style, it would be as much work to rewrite some ArchivePopUp.module.scss as it would be to add it. Since we've already decided to use the same card format for now, there's no reason to not maintain concision. Blog cards are inherently different in content, and thus similarities between blog and archive/event cards are negligible in the first place; archived events are derived from events content-wise. |
Revisions of archive page look good. May want to change the font size of "No Recording"/"No Slides" to that of time/location on event cards (i.e. smaller). |
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.
Approved with @zhamic7's feedback!
Summary
Closes #
Test Plan