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

Display estimated time on resources #8111

Closed
marcellamaki opened this issue May 25, 2021 · 9 comments
Closed

Display estimated time on resources #8111

marcellamaki opened this issue May 25, 2021 · 9 comments
Labels
P1 - important Priority: High impact on UX

Comments

@marcellamaki
Copy link
Member

Overview:
Resources should have a display of the estimated time to completion, if the metadata exists

Done when:

  • Estimated time displays for any resource that has this metadata
  • Explanatory tooltip displays on hover

Beyond scope of this ticket:

  • Setting up estimated time metadata

Figma reference:
Screen Shot 2021-05-25 at 5 10 53 PM

@marcellamaki marcellamaki added the P1 - important Priority: High impact on UX label May 25, 2021
@marcellamaki marcellamaki self-assigned this Jun 8, 2021
@marcellamaki marcellamaki removed their assignment Aug 9, 2021
@nucleogenesis
Copy link
Member

@jtamiace

image

As it was implemented, there is no left-right padding for resources. I hadn't noticed this in the design when reviewing the PR that implemented this. So in order to align the estimated time with the content, it is flush with the side.

Should the resource have padding around it centering it in the screen? Should content have a max-width or should it generally go as wide as the screen?

@jtamiace
Copy link
Contributor

jtamiace commented Sep 7, 2021

Sorry @nucleogenesis, I overlooked that time element when I made the suggestion for the content renderer to maximize use of the available screen space. For videos, EPUBs, and exercises, I agree it would be good to add padding as specced here. Also, the time can be inserted underneath those renderers.

HTML5 and PDF can take up the width and height of the screen, and we can use a smaller fixed bottom bar for the time there.

The downside I'm seeing is that it might be a bit more work to customize it per renderer. Open to suggestions for more consistent representation of time.

time

@rtibbles
Copy link
Member

rtibbles commented Sep 7, 2021

Note that the HTML5 renderer in this mockup is missing the toolbar that the 'fullscreen' button is found in, and the epub renderer here is similarly missing its top toolbar.

I think the renderers can implement their preferred spacing internally, so as to avoid making the content page have hard coded logic about the different renderers and their behaviours, but I am a little surprised that we are choosing to put the time in a completely separate location to all other content metadata.

Adding a bottom bar, especially for content types like HTML5 or PDFs where the most immersive behaviour is to give the renderer free reign over the page below the top bar, feels like it is going to cause nested scrolling behaviours that we had been trying to get away from with the more immersive renderer experience.

@rtibbles
Copy link
Member

rtibbles commented Sep 7, 2021

(apologies for not bringing this up sooner, but I had either completely failed to notice this placement, or had not previously considered the implications for content that might extent below the fold).

@jtamiace
Copy link
Contributor

jtamiace commented Sep 7, 2021

Yeah, it's good to revisit the value to the learner in having the time estimate visible for different learning activities.

We've done it intentionally for the updated quiz renderer, and could see the value for practice resources ("Am I completing this in a good amount of time, compared to how I'd do so with a real quiz/exam in class?"), but even then, time doesn't seem as important if they're just practicing concepts, especially with the KA-style mastery model.

For all other resource types, I can't think of a reason it would be useful to have it that prominent once they're in the renderer... the least so for video/audio.

@jtamiace
Copy link
Contributor

jtamiace commented Sep 7, 2021

The specs for the info side panel already have time included, so here's padding for the different types

renderer

@nucleogenesis
Copy link
Member

nucleogenesis commented Sep 13, 2021

@jtamiace what are your thoughts on changing how we label the estimated time? Radina noted a11y concerns with tooltips and I was thinking that given the space, we could just put the label directly.

Thank you for the updated specs!

@rtibbles agreed that the renderers should handle their own spacing.

@nucleogenesis
Copy link
Member

#8458 does not close this issue but is a response to the above comment by Jessica.

This issue still exists but sounds like there is a plan to put it into the Side Panel - per #8307 so that will depend on this issue.

@nucleogenesis nucleogenesis removed their assignment Sep 22, 2021
@nucleogenesis
Copy link
Member

Closed by #8545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 - important Priority: High impact on UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants