-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
Conversation
There was a problem hiding this 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.
Yes, i am on it. |
@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! 🙂 |
I have removed the duplicate call to the component and a console log. |
@HelNershingThapa done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HelNershingThapa yes sure! I will finish up these things. |
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 |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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. |
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. |
@HelNershingThapa Sure, Everything is fine except the failing tests. I will do as you said and let you know if it worked. |
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. |
Sure @HelNershingThapa I am on it! |
Done! |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Continued by #6325 |
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 🙂