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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LevelDetailsDialog and implement three level types #39087

Merged
merged 6 commits into from
Feb 18, 2021

Conversation

bethanyaconnor
Copy link
Contributor

@bethanyaconnor bethanyaconnor commented Feb 17, 2021

In an effort to avoid a 600+ line PR, I realized I could break the LevelDetails work into multiple PRs using Storybook. This means that this PR doesn't connect this dialog to the lesson plans.

This PR implements three level types:
StandaloneVideo:
Screen Shot 2021-02-16 at 2 50 32 PM

External:
Screen Shot 2021-02-16 at 2 50 42 PM

LevelGroup (text adapted from CurriculumBuilder):
Screen Shot 2021-02-16 at 4 15 35 PM

Links

Testing story

This PR is all testing 馃槄 . Added both Storybook and unit tests.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@bethanyaconnor bethanyaconnor marked this pull request as ready for review February 17, 2021 16:01
@bethanyaconnor bethanyaconnor requested a review from a team as a code owner February 17, 2021 16:01
@bethanyaconnor bethanyaconnor requested review from dmcavoy, Hamms and a team and removed request for dmcavoy and Hamms February 17, 2021 16:01
handleClose: PropTypes.func.isRequired
};

getContentComponent = level => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: getComponentContent

Copy link
Contributor

@dmcavoy dmcavoy left a comment

Choose a reason for hiding this comment

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

馃帀 Yay for storybook!

This is a great start. Can't wait to see more

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Nice progress! I noticed that the "popover" in the spec shows the workspace header (insructions, help & tips, etc). Not sure if this is just future work, but seeing it would really help me place that the content shown here is the instructions of the level (if that is in fact what it is).

expect(loadVideoSpy.calledOnce).to.be.true;
expect(wrapper.contains('Some things to think about.')).to.be.true;
});
});
Copy link
Member

Choose a reason for hiding this comment

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

test coverage looks good!

@bethanyaconnor
Copy link
Contributor Author

@davidsbailey Most levels will have the instructions in the popover but it's been proving a bit tricky so I thought it would be easier to get some of the simpler code in earlier.

Here's a work in progress shot of what it will likely look like when everything is done!
Screen Shot 2021-02-17 at 12 01 38 PM

@dmcavoy
Copy link
Contributor

dmcavoy commented Feb 17, 2021

@davidsbailey Most levels will have the instructions in the popover but it's been proving a bit tricky so I thought it would be easier to get some of the simpler code in earlier.

Here's a work in progress shot of what it will likely look like when everything is done!
Screen Shot 2021-02-17 at 12 01 38 PM

Its probably also worth pointing out that all the level types in this PR do not use a the instructions area currently on the levels themselves

@davidsbailey
Copy link
Member

Thanks @bethanyaconnor and @dmcavoy , all makes sense!

@bethanyaconnor bethanyaconnor merged commit 13ebfc5 into staging Feb 18, 2021
@bethanyaconnor bethanyaconnor deleted the level-details-component-storybook branch February 18, 2021 01:03
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.

None yet

3 participants