feat: open instructor tool in new tab #14
Conversation
| <div data-testid="review_dash"> | ||
| {exam && <iframe title="lti_tool" src={getLaunchUrlByExamId(exam.id)} width="100%" height="1100" style={{ border: 'none' }} />} | ||
| <div> | ||
| {!process.env.LTI_INSTRUCTOR_EMBED && exam && window.open(getLaunchUrlByExamId(exam.id), '_blank')} |
There was a problem hiding this comment.
defaults to opening in new tab (based on env settings) and keeps logic below to embed as iframe.
varshamenon4
force-pushed
the
varshamenon4/instructor-tool-new-tab
branch
from
5642df3
to
b4ab396
Compare
| <div data-testid="review_dash"> | ||
| <div> | ||
| {!ltiToolEmbed && exam && ( | ||
| // todo: add spacing |
There was a problem hiding this comment.
Right now, there is no spacing/padding around the button (see screenshot). I tried by adding classname='p-0' to the button based on similar examples, but I'm now wondering whether this spacing should be added to the component above (i.e. exams page or just the review dashboard)
There was a problem hiding this comment.
I'd look at paragon for layout options and/or a containing element for the button. Maybe https://paragon-openedx.netlify.app/components/actionrow/?
There was a problem hiding this comment.
Added some padding.
Codecov Report
@@ Coverage Diff @@
## main #14 +/- ##
==========================================
+ Coverage 74.73% 75.08% +0.34%
==========================================
Files 22 22
Lines 285 289 +4
Branches 27 29 +2
==========================================
+ Hits 213 217 +4
Misses 69 69
Partials 3 3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| // By default, launch instructor tool in a new tab. Otherwise, embed in dashboard as iframe. | ||
| return ( | ||
| <div data-testid="review_dash"> | ||
| <div style={{ padding: 10 }}> |
There was a problem hiding this comment.
Figured out how to add some padding here without needing to use the Container (the behavior is the same). The only thing I'm unsure about from a best practice standpoint is whether there is a better way to not "hard code" the padding and have it be flexible.
There was a problem hiding this comment.
I reached out to Gabe to see if he could point me to the right place but also okay with leaving it as is as it does work! I don't know if it's overkill and better to just merge/adjust later if needed.
There was a problem hiding this comment.
in general I'd say avoid inline styling if possible but this is pretty small, let's just get this in since Proctorio devs can't launch this page w/o the fix.
varshamenon4
force-pushed
the
varshamenon4/instructor-tool-new-tab
branch
from
a03a365
to
bae1d12
Compare
There was a problem hiding this comment.
I think we'll also need the link in the ReviewExamAttemptButton to open in a new tab as well, which I think may be easily done by setting the "target" param in the button (relevant doc: https://paragon-openedx.netlify.app/components/hyperlink/#props-api)
MST-2074
Rather than viewing the instructor tool in an iframe from the review dashboard, there is now a button that links to the tool and opens it in a new window.