Merge main into nextjs branch
#1657
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* testing codespace fix * testing fix
…rk with course/program datafiles (#1587)
* adding success variant * adding working version * adding working version * fixing text and adding margin to buttons * lint fixes * lint fix * fixing test cases * closing popup before posting data * removing redundancy * removing redundancy * removing empty test * updating styles * updating styles * changes to match design * changes to match design
…urn instead of raise for replaced tasks (#1608)
* adding display label and migration to percolate queries * regenerating specs * adding test and method for returning label * fixing migration * adding display label to admin * adding test for source description * fixing search conditionals in test
This reverts commit 3a906b8.
Adds event capturing for facet selection and search term updating, so we can track these more efficiently in PostHog.
* apply StyledMITLogoLink styles directly to img element * fix app container padding based on header height
* remove border and shadow * remove nav menu button hover state
* adding initial button * adding working dialog * fixing typecheck lint * fixing styles and checking for existing subscriptions * adding tests * fixing lint * adding another test. fixing dialog header * fixing lint * remove trailing question mark
* updating email template with new logo * adding copy update for unit * adding trailing slash * adding trailing slash
| * | ||
| * See https://stackoverflow.com/a/11126701/2747370 | ||
| */ | ||
| display: "block", |
Contributor
Author
There was a problem hiding this comment.
On main, the parent link is display: flex which causes its children to be block. This was the only link styling on main.
On nextjs, that styling was removed. But images are inline elements by default, which messes up their positioning a little. So this makes them block.
| )((props) => { | ||
| const { size, responsive } = { ...props, ...defaultProps } | ||
| return [ | ||
| buttonStyles(props), |
Contributor
Author
There was a problem hiding this comment.
Previous, ActionButton was:
const ActionButton = styled(
React.forwardRef<HTMLButtonElement, ActionButtonProps>((props, ref) => (
<ButtonStyled ref={ref} type="button" {...props} />
))(...)
// and, essentially
const ActionButtonLink = ActionButton.withComponent(Link)This caused ActionButtonLink to be missing the styles from ButtonStyled.
Moving the button styles to its own helper fixed that. Story added.
- apply actionbuttonlink styles correctly - fix logo sizes - fix type errors - regenerate openapi spec - move ecommerce to nextjs
ChristopherChudzicki
force-pushed
the
cc/nextjs-w-main
branch
from
4846047 to
9deded5
Compare
ChristopherChudzicki
added
the
Needs Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
For https://github.com/mitodl/hq/issues/5395
Description (What does it do?)
Merges
mainintonextjsand fixes a few issues that arose.Reviewing:
Suggestion: Most of the commits on this branch are already on
main. The merge conflicts were resolved in c8696de.Some things were broken after the conflict resolutions, so I fixed them in the followup commit.
Most of the changes I made in the followup commit are related to making sure the header and footer look identical on https://learn.mit.edu/ vs the nextjs branch. See video below, which switches between two browser tabs.
Screen.Recording.2024-10-07.at.11.05.34.AM.mov
How can this be tested?
Run locally. You should get recent changes from
main. Notably:/cart./cartrequires setting a posthog flag. See Adds base infra for the Unified Ecommerce frontend #1634