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

Add Button to Student Facing Lesson Plans on Script Overview Page #39202

Merged
merged 16 commits into from
Mar 5, 2021

Conversation

JillianK
Copy link
Contributor

This adds buttons to the student facing lesson plan on the script overview page for lessons that have a lesson plan and scripts that have include_student_lesson_plans set to true.
This is what the script overview page looks like for students
Screen Shot 2021-02-23 at 2 53 35 PM

For teachers:
Screen Shot 2021-02-23 at 3 05 09 PM

Links

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

@JillianK JillianK requested a review from a team as a code owner February 23, 2021 23:36
…code-dot-org into student-lesson-plan-part-3
@dmcavoy
Copy link
Contributor

dmcavoy commented Feb 26, 2021

This is looking really nice Jillian!

I slacked you a question about what we want to happen when a lesson is lockable and has a lesson plan. Is the button still clickable?

Also don't forget to pull in the UI test from the earlier PR and update it to add in click these buttons.

@JillianK
Copy link
Contributor Author

JillianK commented Mar 1, 2021

This is looking really nice Jillian!

I slacked you a question about what we want to happen when a lesson is lockable and has a lesson plan. Is the button still clickable?

Also don't forget to pull in the UI test from the earlier PR and update it to add in click these buttons.

Oh I think the button was still clickable, I just added disabling the button if the lesson is locked.

@dmcavoy
Copy link
Contributor

dmcavoy commented Mar 3, 2021

This is looking really nice Jillian!
I slacked you a question about what we want to happen when a lesson is lockable and has a lesson plan. Is the button still clickable?
Also don't forget to pull in the UI test from the earlier PR and update it to add in click these buttons.

Oh I think the button was still clickable, I just added disabling the button if the lesson is locked.

Can you still navigate to the student lesson plan from other student lesson plans using the dropdown or directly through the URL

@JillianK
Copy link
Contributor Author

JillianK commented Mar 3, 2021

This is looking really nice Jillian!
I slacked you a question about what we want to happen when a lesson is lockable and has a lesson plan. Is the button still clickable?
Also don't forget to pull in the UI test from the earlier PR and update it to add in click these buttons.

Oh I think the button was still clickable, I just added disabling the button if the lesson is locked.

Can you still navigate to the student lesson plan from other student lesson plans using the dropdown or directly through the URL

I think so :( This is probably an ability.rb change right.

…code-dot-org into student-lesson-plan-part-3
@dmcavoy
Copy link
Contributor

dmcavoy commented Mar 3, 2021

This is looking really nice Jillian!
I slacked you a question about what we want to happen when a lesson is lockable and has a lesson plan. Is the button still clickable?
Also don't forget to pull in the UI test from the earlier PR and update it to add in click these buttons.

Oh I think the button was still clickable, I just added disabling the button if the lesson is locked.

Can you still navigate to the student lesson plan from other student lesson plans using the dropdown or directly through the URL

I think so :( This is probably an ability.rb change right.

I think its a change to the controller, we might not need to do it in ability.rb. I think we don't need to block this PR on this issue. I can take a follow up task here to figure out how we want to lock this down. https://codedotorg.atlassian.net/browse/PLAT-792

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.

I think this PR is ready for team review once you pull in the UI test from the previous PR and update to add the clicking of the button. Let me know if you want me to make those updates.

@JillianK JillianK requested a review from a team March 4, 2021 21:30
Base automatically changed from student-lesson-plan-routes to staging March 5, 2021 02:06
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