-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: use frontend-component-header, move away from old studio styles #74
refactor: use frontend-component-header, move away from old studio styles #74
Conversation
3754d8a
to
f7084fb
Compare
Codecov ReportBase: 49.07% // Head: 48.61% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #74 +/- ##
==========================================
- Coverage 49.07% 48.61% -0.46%
==========================================
Files 76 76
Lines 1997 1989 -8
Branches 373 367 -6
==========================================
- Hits 980 967 -13
- Misses 983 988 +5
Partials 34 34
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
it('Fetches block LTI URL to clipboard', async () => { | ||
const library = libraryFactory({ allow_lti: true }); | ||
const blocks = makeN(blockFactoryLine([], { library }), 2); | ||
// todo: figure out how LTI stuff works |
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.
@arbrandes i'm not sure how to test this. i'll gladly bring back the buttons and uncomment these tests once i know how to set up a library to have LTI stuff
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.
Let's push the LTI stuff back until the rest is done - maybe even further. It requires backend fixes before we can revisit the frontend properly.
f7084fb
to
b788d97
Compare
@brian-smith-tcril, tested it, and it (still) works great! Can you please rebase on master? Also, it would be great if we didn't have to merge with a failing |
e4c0074
to
1c8512b
Compare
1c8512b
to
a10b3d6
Compare
@arbrandes this is ready for eyes again i updated the header PR and created a testing branch that updates the |
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.
@brian-smith-tcril, merged the frontend-component-header PR. Tested it in the process of testing this (via your test branch). Looks great! Let me know if we can merge it as is, or if we need a version bump for frontend-component-header
.
@arbrandes a version bump on the header repo would be ideal. just for peace of mind i'd like to verify everything is working in the code we're about to merge before hitting the button |
a10b3d6
to
2814965
Compare
this PR has all the changes from #69
the differences are:
package.json
points to thefrontend-component-header
from npm instead of from my fork