Skip to content
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

DOP-4283: Add buttons for lab drawer #997

Merged
merged 21 commits into from
Feb 2, 2024
Merged

DOP-4283: Add buttons for lab drawer #997

merged 21 commits into from
Feb 2, 2024

Conversation

rayangler
Copy link
Collaborator

@rayangler rayangler commented Jan 30, 2024

Stories/Links:

DOP-4283

Current Behavior:

Server prod

Staging Links:

Server staging with feature flag ON - with drawer
Server staging with feature flag ON - no drawer

The following should load similar to the current behavior:
Server staging with feature flag OFF
Server staging with feature flag OFF

Notes:

  • Main focus of this PR is to add the buttons needed for the lab drawer, update leftover styling for the drawer, and update tests.
  • There were a few changes in plans in both the code and original A/C (see my comments below). This shouldn't affect the implementation too much.
  • To test locally, ensure you have the GATSBY_FEATURE_LAB_DRAWER frontend feature flag set to true, and pull in the latest changes of the parser to parse a page with an instruqt directive with the drawer option. For example:
.. instruqt:: /mongodb-docs/tracks/example?token=example
   :title: Example Lab
   :drawer:

))}
</Template>
</FootnoteContext.Provider>
<InstruqtProvider hasLabDrawer={page?.options?.instruqt}>
Copy link
Collaborator Author

@rayangler rayangler Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to move this away from RootProvider because the isOpen state was being preserved when switching between pages. Moving it to DocumentBody ensures that values are reset between pages since every page has its own instance of the DocumentBody component, rather than the shared behavior of the RootProvider in the layout components.

An alternative solution was to use a useEffect in the Instruqt context to set the state back to default whenever the page slug prop changed value, but it seemed like unnecessary logic at the root level when the context could have just been brought down to the page level. Please let me know if there's a preference though!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me!

return (
<div className={cx(buttonContainerStyle)}>
<Icon className={cx(iconStyle)} width={16} height={16} onClick={handleDrawerSize} glyph={drawerSizeGlyph} />
<Icon className={cx(iconStyle)} width={24} height={24} onClick={handleMaximizeDrawer} glyph="FullScreenEnter" />
Copy link
Collaborator Author

@rayangler rayangler Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, this was supposed to open the Instruqt lab in a new tab, but it turns out I made a mistake during the initial investigation of how lab progress was saved. Tl;dr is that lab progress does not seem to be saved between tabs when the labs are started on different origins (MongoDB vs. Instruqt). Instead, we'll go with the alternative of setting the height of the drawer to its maximum.

If we figure out how to safely preserve lab progress between tabs from different site origins, it should be low effort to readjust this button. Otherwise, I'll go ahead and add a comment to the ticket to reflect these recent findings and this change in functionality accordingly

@rayangler rayangler marked this pull request as ready for review January 31, 2024 21:31
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. minor comment on what the maximize button would do when the drawer is already maximized. CMIIW but seems like design was not updated either

src/components/Instruqt/DrawerButtons.js Outdated Show resolved Hide resolved
@rayangler rayangler merged commit b69f260 into main Feb 2, 2024
2 checks passed
@rayangler rayangler deleted the DOP-4283 branch February 2, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants