-
Notifications
You must be signed in to change notification settings - Fork 25
[WIP] [Don't Merge] Utilize frontend-component-header for studio header #69
Changes from 79 commits
919aad7
6fa0d23
2b86193
305b252
1169f64
5044fca
d8c484d
9d35b9c
2946b63
eacec82
d18cabd
a9f90c1
62abab5
a447a5c
72703ee
8fad5bf
28a6190
7b84c3f
a1c830e
587bbfa
f62ec7e
7af6c5a
5dcf86a
c8e4496
947bafe
c1c3e85
8341d60
38d77af
2479fde
847d028
4d011fd
093feae
76674cc
f934987
521c009
aef0d0f
5c0419f
07e9fbf
ede0b85
da0281f
f2067a0
19f0f12
a811840
8e1c55c
143a668
2eb13dd
43706b0
fc6ed09
e67c42e
c28d989
448dd6e
2cb8643
fec2a07
dae194e
2d44f03
5b0e15f
265ec51
4ed3dea
ba43a3b
0fb9827
be4c66e
aee25bf
1919925
b83cc48
dcb28a1
f048454
ff8d45a
b56b7d2
9edd6f5
42c82d4
1263b90
866d08a
79fb801
2b8d650
37b5cc3
1549782
71f1e47
ee12c7d
e0f4a5e
744d97d
e38d674
fbef590
720bc0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,8 +37,9 @@ | |
"dependencies": { | ||
"@edx/brand": "npm:@edx/[email protected]", | ||
"@edx/frontend-component-footer": "10.2.1", | ||
"@edx/frontend-platform": "1.15.2", | ||
"@edx/paragon": "19.25.3", | ||
"@edx/frontend-component-header": "github:brian-smith-tcril/frontend-component-header#studio-header-for-prs", | ||
"@edx/frontend-platform": "^3.0.0", | ||
"@edx/paragon": "^20.11.1", | ||
"@fortawesome/fontawesome-svg-core": "1.2.36", | ||
"@fortawesome/free-brands-svg-icons": "5.15.4", | ||
"@fortawesome/free-regular-svg-icons": "5.15.4", | ||
|
@@ -83,6 +84,7 @@ | |
"reactifex": "1.1.1", | ||
"redux-mock-store": "1.5.4", | ||
"rosie": "2.0.1", | ||
"webpack-cli": "^4.10.0", | ||
"webpack-merge": "5.8.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,25 +20,31 @@ import { | |
LibraryEditPage, | ||
LibraryListPage, | ||
LibraryCreatePage, | ||
StudioHeader, | ||
LibraryAccessPage, | ||
LibraryAuthoringPage, | ||
StudioHeaderWrapper, | ||
} from './library-authoring'; | ||
import './index.scss'; | ||
import './assets/favicon.ico'; | ||
|
||
mergeConfig({ | ||
LIB_AUTHORING_BASE_URL: process.env.BASE_URL, | ||
STUDIO_BASE_URL: process.env.STUDIO_BASE_URL, | ||
LOGO_URL: process.env.LOGO_TRADEMARK_URL, | ||
BLOCKSTORE_COLLECTION_UUID: process.env.BLOCKSTORE_COLLECTION_UUID, | ||
SECURE_ORIGIN_XBLOCK_BOOTSTRAP_HTML_URL: process.env.SECURE_ORIGIN_XBLOCK_BOOTSTRAP_HTML_URL, | ||
}); | ||
|
||
subscribe(APP_READY, () => { | ||
ReactDOM.render( | ||
<AppProvider store={store}> | ||
<div className="wrapper"> | ||
<StudioHeader /> | ||
<main> | ||
{/* todo: look for a better way to do this, using these routes in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interested in hearing suggestions for alternatives. I assume you're wrapping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my main issue with this implementation is having multiple route switches in the i did add it in f934987 to clear the state (since i was hoping i'd have some better ideas by now, but nothing is coming to mind There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For what it's worth, it seems to me like a legitimate use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, i'll remove the todo |
||
a switch feels less than ideal */} | ||
<Switch> | ||
<Route path={ROUTES.Detail.HOME} component={StudioHeaderWrapper} /> | ||
<Route path="*" component={StudioHeaderWrapper} /> | ||
</Switch> | ||
<main className="library-authoring__main-content"> | ||
<Switch> | ||
<Route exact path={ROUTES.List.HOME} component={LibraryListPage} /> | ||
<Route exact path={ROUTES.List.CREATE} component={LibraryCreatePage} /> | ||
|
@@ -56,7 +62,6 @@ subscribe(APP_READY, () => { | |
</main> | ||
<AboutLibrariesHyperlink /> | ||
<Footer /> | ||
</div> | ||
</AppProvider>, | ||
document.getElementById('root'), | ||
); | ||
|
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.
Thanks for this. Makes it much easier to test.
Do you mind making a PR out of the
frontend-component-header
changes, though? However WIP it still is.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.
openedx/frontend-component-header#264
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.
Excellent, thank you!