[BD-46] fix `Tabs` not resizing on theme switch #2962

viktorrusakov · 2023-12-22T08:03:21Z

Description

Fixes useIndexOfLastVisibleChild hook to observer resizing of container element instead of whole window. This allows to correctly render More... tab on theme switch.

Deploy Preview

https://deploy-preview-2962--paragon-openedx.netlify.app/components/tabs/

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

openedx-webhooks · 2023-12-22T08:03:25Z

Thanks for the pull request, @viktorrusakov!

When this pull request is ready, tag your edX technical lead.

netlify · 2023-12-22T08:03:26Z

✅ Deploy Preview for paragon-openedx ready!

Name	Link
🔨 Latest commit	`a734575`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/65854900b3bf3600089bf70c
😎 Deploy Preview	https://deploy-preview-2962--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

viktorrusakov · 2023-12-22T08:16:08Z

src/hooks/useIndexOfLastVisibleChild.mdx

-  const containerElementRef = React.useRef(null);
+  const [containerElementRef, setContainerElementRef] = React.useState(null);


This is technically a breaking change for consumers, but I don't see any usage of this hook in Usage Insighhts table, so I think it should be okay to release it as a fix.

The reason we need to use callback ref is to trigger a re-render once the ref is attached to the DOM element, previously this re-render was caused by useWindowSize hook that was used in useIndexOfLastVisibleChild hook before this PR.

[inform] Yeah, it looks like there aren't any consumers of this hook from Paragon directly, but there are (nearly) duplicate functions that exist within frontend-app-learning (source) and frontend-app-discussions (source). They were likely implemented before we implemented the responsive Tabs in Paragon.

I agree this should probably be OK to release a fix.

…stead of window resize

codecov · 2023-12-22T08:40:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9a16ba1) 92.87% compared to head (a734575) 92.88%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2962   +/-   ##
=======================================
  Coverage   92.87%   92.88%           
=======================================
  Files         235      235           
  Lines        4268     4272    +4     
  Branches     1033     1033           
=======================================
+ Hits         3964     3968    +4     
  Misses        300      300           
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adamstankiewicz · 2023-12-22T13:40:48Z

src/hooks/useIndexOfLastVisibleChild.mdx

-  const containerElementRef = React.useRef(null);
+  const [containerElementRef, setContainerElementRef] = React.useState(null);


[inform] Yeah, it looks like there aren't any consumers of this hook from Paragon directly, but there are (nearly) duplicate functions that exist within frontend-app-learning (source) and frontend-app-discussions (source). They were likely implemented before we implemented the responsive Tabs in Paragon.

I agree this should probably be OK to release a fix.

openedx-semantic-release-bot · 2023-12-22T13:56:16Z

🎉 This PR is included in version 21.12.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-semantic-release-bot · 2023-12-27T10:40:43Z

🎉 This PR is included in version 22.0.0-alpha.24 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-webhooks added the blended PR is managed through 2U's blended developmnt program label Dec 22, 2023

viktorrusakov linked an issue Dec 22, 2023 that may be closed by this pull request

Tabs "More..." dropdown seems to expand beyond Tabs bottom border #2445

Closed

viktorrusakov requested review from monteri, PKulkoRaccoonGang and adamstankiewicz December 22, 2023 08:12

viktorrusakov commented Dec 22, 2023

View reviewed changes

fix: observe container resizing in useIndexOfLastVisibleChild hook in…

a734575

…stead of window resize

viktorrusakov force-pushed the rusakov/fix-tabs branch from 21af7ca to a734575 Compare December 22, 2023 08:29

PKulkoRaccoonGang approved these changes Dec 22, 2023

View reviewed changes

adamstankiewicz approved these changes Dec 22, 2023

View reviewed changes

adamstankiewicz merged commit dc38d63 into master Dec 22, 2023
10 checks passed

adamstankiewicz deleted the rusakov/fix-tabs branch December 22, 2023 13:43

openedx-semantic-release-bot added the released label Dec 22, 2023

openedx-semantic-release-bot added the released on @alpha label Dec 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-46] fix `Tabs` not resizing on theme switch #2962

[BD-46] fix `Tabs` not resizing on theme switch #2962

viktorrusakov commented Dec 22, 2023 •

edited

Loading

openedx-webhooks commented Dec 22, 2023

netlify bot commented Dec 22, 2023 •

edited

Loading

viktorrusakov Dec 22, 2023 •

edited

Loading

adamstankiewicz Dec 22, 2023

codecov bot commented Dec 22, 2023

adamstankiewicz Dec 22, 2023

openedx-semantic-release-bot commented Dec 22, 2023

openedx-semantic-release-bot commented Dec 27, 2023

		const containerElementRef = React.useRef(null);
		const [containerElementRef, setContainerElementRef] = React.useState(null);

[BD-46] fix Tabs not resizing on theme switch #2962

[BD-46] fix Tabs not resizing on theme switch #2962

Conversation

viktorrusakov commented Dec 22, 2023 • edited Loading

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

openedx-webhooks commented Dec 22, 2023

netlify bot commented Dec 22, 2023 • edited Loading

✅ Deploy Preview for paragon-openedx ready!

viktorrusakov Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

adamstankiewicz Dec 22, 2023

Choose a reason for hiding this comment

codecov bot commented Dec 22, 2023

Codecov Report

adamstankiewicz Dec 22, 2023

Choose a reason for hiding this comment

openedx-semantic-release-bot commented Dec 22, 2023

openedx-semantic-release-bot commented Dec 27, 2023

[BD-46] fix `Tabs` not resizing on theme switch #2962

[BD-46] fix `Tabs` not resizing on theme switch #2962

viktorrusakov commented Dec 22, 2023 •

edited

Loading

netlify bot commented Dec 22, 2023 •

edited

Loading

viktorrusakov Dec 22, 2023 •

edited

Loading