feat: expose containerProps in StudioFooter [FC-0062] #463

rpenido · 2024-09-05T19:48:28Z

Description

This PR adds a new boolean containerProps property to the StudioFooter component. This property will be passed downstream to the Container component in the Footer, allowing the user to override the Container props (i.e. changing the max size).

More information

Part of:

Adjustments to tile responsiveness when sidebar is opened frontend-app-authoring#1221

Depends on:

feat: migrates Container to TypeScript; Container without max-width on docs site [FC-0062] paragon#3216

Private ref: FAL-3820

openedx-webhooks · 2024-09-05T19:48:32Z

Thanks for the pull request, @rpenido!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently unmaintained.

To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:

On the right-hand side of the PR, find the Contributions project, click the caret in the top right corner to expand it, and check the "Primary PM" field for the name of your PM.
Find their GitHub handle here.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2024-09-05T19:51:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.22%. Comparing base (2d5f5fe) to head (c164f37).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
+ Coverage   90.74%   91.22%   +0.48%     
==========================================
  Files           5        5              
  Lines          54       57       +3     
  Branches       18       19       +1     
==========================================
+ Hits           49       52       +3     
  Misses          5        5

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

pomegranited

Same comments as on openedx/frontend-component-header#529 (review) :)

pomegranited · 2024-09-06T06:25:53Z

src/components/studio-footer/StudioFooter.jsx

 const StudioFooter = ({
-  // injected
-  intl,
+  full,


Same comment as https://github.com/openedx/frontend-app-course-authoring/pull/1258/files#diff-1c1e7c5912d6ff798a72899cde884c54211549e22fec1f469c8a5a4a854819e1R20 -- can this be named fullWidth or fullSize instead?

Done: 80d05dc

pomegranited · 2024-09-06T06:26:45Z

src/components/studio-footer/StudioFooter.jsx

        <div className="col border-top ml-2" />
      </div>
-      <Container size="xl" className="px-4">
+      <Container size={full ? undefined : 'xl'} className="px-4">


If we add a size="full" option instead, then this becomes a lot clearer:

Suggested change

<Container size={full ? undefined : 'xl'} className="px-4">

<Container size={full ? 'full': 'xl'} className="px-4">

Check my comment in the Paragon PR

pomegranited

👍

I tested this while testing openedx/frontend-app-authoring#1258 (review)
I read through the code
~~I checked for accessibility issues~~ N/A
~~Includes documentation~~ N/A
~~User-facing strings are extracted for translation~~ N/A

pomegranited · 2024-09-19T04:43:33Z

src/components/studio-footer/StudioFooter.jsx

+  containerProps: Container.propTypes,
+};
+
+StudioFooter.defaultProps = {


containerProps should be optional, and/or populated with a default value, correct?

Fixed here: a996f11

pomegranited

Works perfectly thank you @rpenido ! 👍

I tested this while testing openedx/frontend-app-authoring#1258
I read through the code
~~I checked for accessibility issues~~ N/A
~~Includes documentation~~ N/A
~~User-facing strings are extracted for translation~~ N/A

rpenido · 2024-09-20T14:04:53Z

Hi @adamstankiewicz! Could you do a review/merge here when you have some time? Thank you!

adamstankiewicz · 2024-10-01T13:29:52Z

src/components/studio-footer/StudioFooter.jsx

-  intl,
+  containerProps,
 }) => {
+  const intl = useIntl();


Nice refactor to use hooks vs. the injectIntl HOC.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 5, 2024

rpenido mentioned this pull request Sep 5, 2024

feat: allow full width content in library authoring [FC-0062] openedx/frontend-app-authoring#1258

Merged

rpenido force-pushed the rpenido/fal-3820-allow-full-width-footer branch 2 times, most recently from ed53162 to 3b9230d Compare September 5, 2024 23:22

rpenido changed the title ~~feat: allow full width footer~~ feat: allow full width footer [FC-0062] Sep 6, 2024

pomegranited suggested changes Sep 6, 2024

View reviewed changes

pomegranited approved these changes Sep 10, 2024

View reviewed changes

rpenido mentioned this pull request Sep 17, 2024

fix: responsiveness on library authoring sidebar [FC-0062] openedx/frontend-app-authoring#1293

Merged

feat: expose containerProps in StudioFooter

36c5fd8

rpenido force-pushed the rpenido/fal-3820-allow-full-width-footer branch from 80d05dc to 36c5fd8 Compare September 18, 2024 22:00

rpenido marked this pull request as ready for review September 18, 2024 22:01

rpenido changed the title ~~feat: allow full width footer [FC-0062]~~ feat: expose containerProps in StudioFooter [FC-0062] Sep 18, 2024

rpenido requested review from adamstankiewicz and pomegranited September 19, 2024 00:07

pomegranited suggested changes Sep 19, 2024

View reviewed changes

fix: add defaultProps to containerProps

a996f11

rpenido force-pushed the rpenido/fal-3820-allow-full-width-footer branch from 23b2254 to a996f11 Compare September 19, 2024 20:04

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Sep 19, 2024

pomegranited approved these changes Sep 19, 2024

View reviewed changes

Merge branch 'master' into rpenido/fal-3820-allow-full-width-footer

6dce26b

rpenido added 2 commits October 1, 2024 10:27

chore: fix dependencies

14804ef

Merge branch 'master' into rpenido/fal-3820-allow-full-width-footer

c164f37

adamstankiewicz approved these changes Oct 1, 2024

View reviewed changes

adamstankiewicz merged commit cc647e7 into openedx:master Oct 1, 2024

	<Container size={full ? undefined : 'xl'} className="px-4">
	<Container size={full ? 'full': 'xl'} className="px-4">

feat: expose containerProps in StudioFooter [FC-0062] #463

feat: expose containerProps in StudioFooter [FC-0062] #463

Uh oh!

Conversation

rpenido commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

More information

Uh oh!

openedx-webhooks commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

Uh oh!

codecov bot commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

pomegranited Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

rpenido Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

pomegranited Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

rpenido Sep 6, 2024

Choose a reason for hiding this comment

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

pomegranited Sep 19, 2024

Choose a reason for hiding this comment

Uh oh!

rpenido Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

rpenido commented Sep 20, 2024

Uh oh!

adamstankiewicz Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rpenido commented Sep 5, 2024 •

edited

Loading

openedx-webhooks commented Sep 5, 2024 •

edited

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

rpenido Sep 19, 2024 •

edited

Loading