iFixit / react-commerce

A work in progress prototype for iFixit e-commerce functionalities.
https://react-commerce.vercel.app
2 stars 0 forks source link

Vulcan: Problem List: Ancestor problem lists #2231

Closed ianrohde closed 7 months ago

ianrohde commented 7 months ago

Issue

Add ancestor Problems to Problems List page when available. See issue description for further details.

Mock-up text is not final. Figure out final copy with Max.

Text will be updated separately in https://github.com/iFixit/ifixit/issues/51815

CR/QA

/Troubleshooting/dryer (no ancestors) /Troubleshooting/AEG_T865901H_dryer (has ancestors)

Closes https://github.com/iFixit/ifixit/issues/51062

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react-commerce ✅ Ready (Inspect) Visit Preview Feb 7, 2024 6:56pm
react-commerce-prod ✅ Ready (Inspect) Visit Preview Feb 7, 2024 6:56pm
ghost commented 7 months ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/iFixit/react-commerce/2231/2b6de9c0/35dec8c6c55dadbc01ebe963cfa2670391899fe8.svg)](https://app.codesee.io/r/reviews?pr=2231&src=https%3A%2F%2Fgithub.com%2FiFixit%2Freact-commerce) #### Legend CodeSee Map legend
github-actions[bot] commented 7 months ago

📦 Next.js Bundle Analysis for @ifixit/commerce-frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

kthaler commented 7 months ago

QA 💎 - When a device has problem pages, and it's ancestor also has problem pages, the device's troubleshooting page will display it's own problem pages, and in a second heading it will show it's ancestor's problem pages.

deploy_block 🦖

  1. What is expected when there are a hierarchy of children with problem pages? Should a child-of-a-child device inherit the base problem pages or it's own parent problem pages, or both? For example, I made a child of the AEG T865901H dryer device and gave it a troubleshooting page: https://me.cominor.com/Device/child_test . In the troubleshooting section of the device page, only the dryer troubleshooting pages are inherited, and not the AEG T865901H dryer troubleshooting page. In the Troubleshooting page of the child device, the AEG T865901H dryer Troubleshooting page isn't there, only the dryer pages.

Text will be updated separately in https://github.com/iFixit/ifixit/issues/51815

Not sure if my issue applies then, so please disregard if this isn't in the scope of the pull. At widths of 767px to 575px, the h2 heading seems too close to the navbar. Video:

https://github.com/iFixit/react-commerce/assets/145375039/2afcfb7b-d2ad-40c2-a61f-4b6ca6bead43

ianrohde commented 7 months ago

At widths of 767px to 575px, the h2 heading seems too close to the navbar.

This should be corrected, thanks!

Fixed in https://github.com/iFixit/react-commerce/pull/2231/commits/4f95bc5e7445c142167023f4a8466f34067ebc47

andyg0808 commented 7 months ago

What is expected when there are a hierarchy of children with problem pages? Should a child-of-a-child device inherit the base problem pages or it's own parent problem pages, or both?

The troubleshooting pages of a device with the inheritance flag are inherited by all its child devices. No pages from child devices are inherited by more distant descendants. For full details, here's the spec: https://github.com/iFixit/ifixit/issues/48260

For example, I made a child of the AEG T865901H dryer device and gave it a troubleshooting page: me.cominor.com/Device/child_test. In the troubleshooting section of the device page, only the dryer troubleshooting pages are inherited, and not the AEG T865901H dryer troubleshooting page.

:+1: That's what we want. We can add the inheritance flag to the AEG T865901H dryer to have its troubleshooting pages also get inherited, at which point the child_test device should have both Dryer and AEG T865901H troubleshooting pages.

un_deploy_block :baby:

deltuh-vee commented 7 months ago

QA 🎬 h2 heading is no longer too close to the navbar. deploy_block ❓ Did you also mean to make this larger? CleanShot 2024-02-07 at 10 22 03@2x

ianrohde commented 7 months ago

@deltuh-vee was that observed on the latest preview? I'm not seeing it: https://react-commerce-prod-gi2hw5yqz-ifixit.vercel.app/Troubleshooting/dryer

It is larger. I'll set the fontSize statically so heading changes won't modify

Details Screenshot 2024-02-07 at 10 46 14 AM
ianrohde commented 7 months ago

Answers heading fontSize fixed in https://github.com/iFixit/react-commerce/pull/2231/commits/35dec8c6c55dadbc01ebe963cfa2670391899fe8

un_deploy_block 🟢

deltuh-vee commented 7 months ago

QA 🎬 Larger text has been fixed.