-
Notifications
You must be signed in to change notification settings - Fork 6
Core 1264 port default layout part 1 #2783
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
base: main
Are you sure you want to change the base?
Conversation
76c81bc
to
bf04c06
Compare
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.
Removed the Welcome code, which was for use with my_openstax, which has been mothballed.
return ( | ||
<div className="page-header"> | ||
<JITLoad importFn={() => import('./sticky-note/sticky-note.js')} stickyData={stickyData} /> | ||
<StickyNote stickyData={stickyData} /> |
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.
Having this load dynamically was not worth the hassle.
import PutAway from '~/components/put-away/put-away'; | ||
import './shared.scss'; | ||
|
||
// Shim for incognito windows that disable localStorage |
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.
Moved this to main.js
await screen.findByText('What is your question about?'); | ||
expect(screen.queryAllByRole('navigation')).toHaveLength(0); | ||
}); | ||
it('handles piTracker ', async () => { |
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 was Pulse Insights. The test fails consistently now, in any case, and removing it doesn't leave code uncovered.
expect(button.parentElement?.classList.contains('active')).toBe(true); | ||
await user.keyboard('{Escape}'); | ||
expect(button.parentElement?.classList.contains('active')).toBe(false); | ||
// If clicked when not active, it does nothing |
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 code is tested in the layouts/default test now.
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've got some large blocks commented out. They're largely tests that Claude wrote and I haven't determined whether they're useful. It think they'll ultimately mostly come out, but will address them in later pieces of the layouts/default port.
7e1e538
to
5bda6f8
Compare
af7c284
to
d52ed1b
Compare
CORE-1264
The first installment of porting layouts/default to TypeScript, covering
footer
header.tsx
header/menus/menus.tsx
header/menus/give-button
header/menus/logo
header/menus/menu-expander
header/menus/upper-menu
header/sticky-note
lower-sticky-note
shared (coverage not quite complete here)