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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion webapp/src/pages/boardPage.scss
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}
}

@media screen and (max-width: 768px) {
@media screen and (max-width: 768px) {
.mobileWarning {display: flex;}
}
}
Expand All @@ -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.

}