mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

feat: add file size in resource list and resource pages #1225

Closed ibrahimjaved12 closed 1 year ago

ibrahimjaved12 commented 1 year ago

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/1681

Description (What does it do?)

Adds file size in resource list pages and individual resource pages. Refer to figma designs in https://github.com/mitodl/hq/issues/1681#issuecomment-1746874289

In this PR, apart from the SCSS changes, we:

How can this be tested?

Test rendering for (to ensure nothing has broken for moving resource partials from course-v2 to base-theme): www online + www offline course online + course offline.

Steps: First checkout to this branch. yarn build /to/ocw-content-rc/ocw-www or a course/ /to/ocw-hugo-projects/ocw-www or ocw-course-v2/config-offline.yaml For offline themes.

yarn start www For online www Make sure the builds succeed ^ from dist directory do check that the theme is alright too from any index pages.

For course, we will want to:

  1. yarn start course 6.0002-fall-2016 (using this course because this has file size content)
  2. Go to: http://localhost:3000/download/
  3. Check the UI.
  4. Click on a resource (other than video lectures)
  5. Check the UI. Also you might want to use bigger resource names (like I have done in the screenshots) to ensure the UI does not break.
  6. Also try different file sizes, too small, too big. Make sure the unit is okay.
  7. Also check for cases where a resource might not have file size (just clear the file_size from the .md file of a resource)

For offline course theme, build a course and go to dist/index.html and then "Browse Resources". Steps will be same as above but not as thorough testing just make sure the changes have applied.

References

Use these figma links for reference. https://github.com/mitodl/hq/issues/1681#issuecomment-1746874289

Screenshots (if appropriate):

https://github.com/mitodl/ocw-hugo-themes/assets/109785089/94d91928-921e-4f2a-b4e3-b6c3a7d7a8cf

image image image image image image image image image image image image
github-actions[bot] commented 1 year ago

Netlify Deployments:
www: https://ocw-hugo-themes-www-pr-1225--ocw-next.netlify.app/
Course v2: https://ocw-hugo-themes-course-v2-pr-1225--ocw-next.netlify.app/

mbilalmughal commented 1 year ago

@ibrahimjaved12 The UI looks good to me. One request, If you could decrease the space between the description and the text, that would be better.

However, as we have discussed, we won't be implementing any additional fixes at this time. For instance, the right-hand space on the mobile container is not equal to the right-hand side.

mbilalmughal commented 1 year ago

@Ferdi Could you please take a look at this? I've reduced the file size label's font size and explained it in this thread

pdpinch commented 1 year ago

@mbilalmughal can you explain what you and @ibrahimjaved12 have changed in response to Ferdi's comment? I don't recall the original design, so I'm not sure what's new.

mbilalmughal commented 1 year ago

@pdpinch, we've changed the file size to a minimum of 10px. Please see the attached image. @ibrahimjaved12 has not pushed the code yet, we are waiting for a response.

Originally, the file size font and the title font were the same, but now the file size font has been reduced to 10px.

image

mbilalmughal commented 1 year ago

Before, also attached with this PR

image

pdpinch commented 1 year ago

@ibrahimjaved12 we've decided to make another pass on the design for this. I expect this will be on hold for at least another week.

ibrahimjaved12 commented 1 year ago

Animated hover video:

https://github.com/mitodl/ocw-hugo-themes/assets/109785089/94d91928-921e-4f2a-b4e3-b6c3a7d7a8cf