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

Fixed a bug causing double scroll bars to appear #2217

Merged
merged 1 commit into from
Feb 2, 2022
Merged

Conversation

harshilsharma63
Copy link
Member

Summary

Fixed a bug causing double scroll bars to appear

Ticket Link

Fixes #2211

@harshilsharma63 harshilsharma63 requested a review from a team as a code owner February 2, 2022 01:15
@harshilsharma63 harshilsharma63 requested review from chenilim and removed request for a team February 2, 2022 01:15
@@ -46,3 +46,7 @@
width: 600px;
height: 468px;
}

.mainContentRow {
height: calc(100% - 40px);
Copy link
Contributor

Choose a reason for hiding this comment

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

@harshilsharma63 should this fix be applied in mattermost/webapp repo? This class name is not used in focalboard sources.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kamre no it should not be in webapp. The issue is only with focalboard and not other products. The main cause of this issue is the sidebar but after spending about half an hour, I couldn't figure out what was causing the issue. This CSS change fixes it, though not the 100% ideal solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

@harshilsharma63 but if the class name mainContentRow will be renamed in mattermost/webapp then this will require to update focalboard repo. Also just changing this 40px will also require an update here in focalboard repo inside this calc expression.

Looks like this styling is tightly coupled to mattermost/webapp repo. And this file boardPage.scss is not even inside the plugin code. So may be the issue with styling should be resolved in mattermost/webapp repo.

One more notice: for Playbooks that height: 100% for mainContentRow doesn't look correct, but root component in case of Playbooks has overflow hidden and there is no scroll bar.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm aware of this. I'll update the styling later. We're making a lot of changes to the sidebar in a separate feature branch so I don't want to spend much time here knowing the whole component is gonna change a few weeks later.

Copy link
Collaborator

@sbishel sbishel left a comment

Choose a reason for hiding this comment

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

Let's merge and fix with new permissions. This should only be in for 1 release.

@sbishel sbishel merged commit 55879bd into main Feb 2, 2022
@sbishel sbishel deleted the fix-double-scroll branch February 2, 2022 20:48
hmhealey added a commit that referenced this pull request Feb 4, 2022
harshilsharma63 pushed a commit that referenced this pull request Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Double vertical scroll bar
3 participants