Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

made the main page static and enabled scrolling in the filters component #5932

Closed

Conversation

mtalhabaig3
Copy link
Contributor

Fixes issue #5884

Hey @HelNershingThapa, I almost fixed the issue but there is one thing left, when we the more filter component appears, the rest of the page becomes static and scrolling is enabled in the more filter component, however the height of the component overflows below the bottom of the screen. Right now i am thinking to calculate the height of the content above and set the height of the filter component 100vh - (above content height), so that it will not overflow and stick to the height available up to the end of the screen. What do you think about it?

Kindly review 🙂

Copy link
Contributor

@HelNershingThapa HelNershingThapa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your proposed solution sounds good to me and is reasonable! Please ensure that the filter component doesn't overflow and remains scrollable within the available height up to the end of the screen.

frontend/src/views/project.css Outdated Show resolved Hide resolved
frontend/src/views/project.js Outdated Show resolved Hide resolved
frontend/src/views/project.js Outdated Show resolved Hide resolved
@mtalhabaig3
Copy link
Contributor Author

Yes, i am on it.

@mtalhabaig3
Copy link
Contributor Author

mtalhabaig3 commented Jun 22, 2023

@HelNershingThapa I have successfully solved the issue. I have set the height of the more filters menu by subtracting the inner window height by the height of the content above it. I have also added the functionality so that the height of the menu changes when we resize the window vertically. Also, I am calling the "More Filter" component twice for testing. Kindly review and let me know if you want some thing changed! 🙂

@mtalhabaig3
Copy link
Contributor Author

I have removed the duplicate call to the component and a console log.

frontend/src/views/project.js Outdated Show resolved Hide resolved
frontend/src/assets/styles/_extra.scss Outdated Show resolved Hide resolved
@mtalhabaig3
Copy link
Contributor Author

@HelNershingThapa done.

Copy link
Contributor

@HelNershingThapa HelNershingThapa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great.

I noticed there is a horizontal scrollbar appearing in the user interface. Could you please remove the scrollbar?

scrollbar

Also, there is a failing test case due to the addition of useEffect code. Could you please address those failing tests as well? Thank you!

@mtalhabaig3
Copy link
Contributor Author

@HelNershingThapa yes sure! I will finish up these things.

@mtalhabaig3
Copy link
Contributor Author

Hi @HelNershingThapa, I have hide the horizontal scroll bar, another issue i noticed was when you open any dropdown inside the filters component and if the dropdown overflows the filters component, the rest f the page scrolls down a little, although the dropdown remains inside the component but still the rest of the page scrolls according to the overflowed length of the dropdown. I have changed the menuPosition in the Select component to "fixed". Now the dropdowns dont overflow the container and the behaviour looks fine. I will address the failing tests

@sonarcloud
Copy link

sonarcloud bot commented Jul 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@HelNershingThapa
Copy link
Contributor

hi @mtalhabaig3 I hope you've been well. It's been a little while since the last update on this PR. Could you please provide an update on its current status? I'm particularly interested in the progress regarding the changes you mentioned in your previous comment, such as addressing the failing tests and ensuring the improvements for the dropdown behavior.

@HelNershingThapa
Copy link
Contributor

If the failing test case is causing a roadblock, you might consider adding the following line to the test:

jest.spyOn(document, 'getElementById').mockReturnValue({ offsetHeight: 100 });

This mock should suffice for now, as it doesn't seem to interfere with other aspects of the test case. Although I'm not a Jest testing expert, this approach might help move things forward without significant hindrance.

Your efforts to resolve these issues are highly valued, and I look forward to seeing this pull request's continued progress.

@mtalhabaig3
Copy link
Contributor Author

@HelNershingThapa Sure, Everything is fine except the failing tests. I will do as you said and let you know if it worked.

@mtalhabaig3 mtalhabaig3 reopened this Aug 17, 2023
@HelNershingThapa
Copy link
Contributor

hi @mtalhabaig3 I did notice a few commented out code that could benefit from some cleanup. Once those are done, I'll gladly dive into the review and merge the code.

@mtalhabaig3
Copy link
Contributor Author

Sure @HelNershingThapa I am on it!

@mtalhabaig3
Copy link
Contributor Author

Done!

@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.5% 0.5% Duplication

@ramyaragupathy ramyaragupathy added this to the v4.7.2 milestone Apr 8, 2024
@royallsilwallz
Copy link
Contributor

royallsilwallz commented Apr 17, 2024

Continued by #6325

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] More filters menu height should extend to the bottom of the page or container should not overflow
4 participants