refactor: improve library sub header by rpenido · Pull Request #1573 · openedx/frontend-app-authoring

rpenido · 2024-12-16T21:11:49Z

Description

This PR changes the Library (and Collection) subheader, according to #1486

Before

After

More Information

Related to Improvements to Library Header #1486

Testing Instruction

Open a Library and check the header
Do the same with a Collection

To test the FilterByBlock change:

Open a library with components and collections
From the Components tab, select a Block Type
Change to the Collections tab and check if the selection is shown on the dropdown

Private ref: FAL-3981

openedx-webhooks · 2024-12-16T21:11:54Z

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 maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

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-12-16T22:02:11Z

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.96%. Comparing base (b110b6b) to head (ef06574).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/search-manager/FilterByBlockType.tsx	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1573      +/-   ##
==========================================
- Coverage   92.96%   92.96%   -0.01%     
==========================================
  Files        1075     1075              
  Lines       21190    21193       +3     
  Branches     4554     4486      -68     
==========================================
+ Hits        19700    19702       +2     
- Misses       1424     1425       +1     
  Partials       66       66

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

rpenido · 2024-12-16T23:41:12Z


      // If the content is less than the screen height, fetch the next page.
-      const hasNoScroll = document.body.scrollHeight <= window.innerHeight;
+      const hasNoScroll = (document.body.scrollHeight - loadLimit) <= window.innerHeight;


This was a small bug that if the last line was half-filled, it was not calling a new fetch

rpenido · 2024-12-16T23:43:51Z

+              // Show applied filter items for block types that are not in the current search results
+              hiddenBlockTypes.map(blockType => <FilterItem key={blockType} blockType={blockType} count={0} />)


I added this feature to show filtered items not in the current view.
Doing the same for the Tags filter is way more complicated.

If you feel this is not necessary, I can remove it.

@rpenido How can we test this manually?

Added to the testing instructions:

Open a library with components and collections

From the Components tab, select a Block Type

Change to the Collections tab and check if the selection is shown on the dropdown

Got it. I think it would be better to just hide the whole types filter option in collections tab since it is always empty.

The issue is that the filter will still be applied if we hide it. So I think it is better to show it.
We could hide and clear this filter criteria by switching to the Collections tab. But I think the way we are doing now (persisting the filter) is better.

What do you think @ChrisChV?

Can't we hide the filter and not apply it, while we're on the collections tab?

At first, I didn't find a good way to do it as the state is stored in the SearchManager. Returning to it today, I found it: #1576

navinkarkera

@rpenido Nice work! 👍

I tested this: (Tested Library page and collection page search and filters)
I read through the code
I checked for accessibility issues
Includes documentation

navinkarkera · 2024-12-17T10:22:26Z

+              // Show applied filter items for block types that are not in the current search results
+              hiddenBlockTypes.map(blockType => <FilterItem key={blockType} blockType={blockType} count={0} />)


@rpenido How can we test this manually?

Co-authored-by: Navin Karkera <navin@opencraft.com>

ChrisChV

@rpenido Looks good 👍 Could you fix the broken validations?

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

rpenido force-pushed the rpenido/fal-3981-improvements-to-library branch 2 times, most recently from d43ae54 to 7e6b0dd Compare December 16, 2024 21:52

feat: improve library sub header

db409cd

rpenido force-pushed the rpenido/fal-3981-improvements-to-library branch from 7e6b0dd to db409cd Compare December 16, 2024 22:14

rpenido commented Dec 16, 2024

View reviewed changes

navinkarkera approved these changes Dec 17, 2024

View reviewed changes

fix: spacing between search and filters

3275c7c

Co-authored-by: Navin Karkera <navin@opencraft.com>

rpenido marked this pull request as ready for review December 17, 2024 16:23

rpenido requested a review from a team as a code owner December 17, 2024 16:23

fix: eslint

ef06574

ChrisChV approved these changes Dec 17, 2024

View reviewed changes

ChrisChV changed the title ~~feat: improve library sub header~~ refactor: improve library sub header Dec 18, 2024

ChrisChV merged commit 230960b into openedx:master Dec 18, 2024

ChrisChV deleted the rpenido/fal-3981-improvements-to-library branch December 18, 2024 17:56

Arpit-Nakrani-Networked pushed a commit to Arpit-Nakrani-Networked/frontend-app-authoring that referenced this pull request Jul 13, 2025

fix: updated notifiations preferences url (openedx#1573)

5e5fdeb

		// Show applied filter items for block types that are not in the current search results
		hiddenBlockTypes.map(blockType => <FilterItem key={blockType} blockType={blockType} count={0} />)

Uh oh!

Conversation

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

Uh oh!

Description

Before

After

More Information

Testing Instruction

To test the FilterByBlock change:

Uh oh!

openedx-webhooks commented Dec 16, 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 Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rpenido Dec 16, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navinkarkera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisChV left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rpenido commented Dec 16, 2024 •

edited

Loading

openedx-webhooks commented Dec 16, 2024 •

edited

Loading

codecov Bot commented Dec 16, 2024 •

edited

Loading

rpenido Dec 16, 2024 •

edited

Loading