Skip to content

fix horizontal scroll#332

Merged
meganrm merged 3 commits intomainfrom
fix/horizontal-scroll
Mar 28, 2022
Merged

fix horizontal scroll#332
meganrm merged 3 commits intomainfrom
fix/horizontal-scroll

Conversation

@meganrm
Copy link
Copy Markdown
Contributor

@meganrm meganrm commented Mar 22, 2022

Problem

On chrome the landing page had horizontal scroll

Solution

I first narrowed it down to the max-width setting on the text content, but it was just an arbitrary cutoff: If i set it to over 1040px, there was no scroll.
So I finally figured out it was the markdown content, we had both column number and height set, which I guess caused an invisible overflow even though the text fit.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Change summary:

  • Moved text content into it's own div
  • Experimented with changing the max width settings
  • Removed the height setting
  • Revert max width to original setting

Steps to Verify:

  1. npm start
  2. open in chrome
  3. should not be able to horizontally scroll on landing page

Screenshots (optional):

Before:
Screen Shot 2022-03-22 at 10 58 16 AM
NOTICE the text is longer in column 2
Screen Shot 2022-03-22 at 12 12 38 PM
After:
Screen Shot 2022-03-22 at 12 12 25 PM

@meganrm meganrm requested a review from a team as a code owner March 22, 2022 19:13
@meganrm meganrm requested review from blairlyons, schoinh and toloudis and removed request for a team March 22, 2022 19:14
biologists by removing major challenges to sharing,
accessing, and comparing simulation results.
</p>
<div className={styles.textContent}>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this looks like a lot of changes but all i did was add a div around the text content instead of styling with .panel * which I initially thought might be causing the problem, but left because it seems safer


.markdown {
column-count: 2;
height: 540px;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the change that fixes the problem (see screen shots for before and after of the acknowledgments)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's what I would have done (I had made this issue for this bug but looks like it got buried - #329)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh man! I totally missed this, I'll be better about searching in the issues in the future. Thanks for reviewing!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 22, 2022

jest coverage report 🧪

Total coverage

Status Category Percentage Covered / Total
🟡 Statements 74.06% 517/698
🟡 Branches 69.07% 105/152
🔴 Functions 42.85% 75/175
🟡 Lines 73.99% 498/673

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold


.markdown {
column-count: 2;
height: 540px;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's what I would have done (I had made this issue for this bug but looks like it got buried - #329)

@meganrm meganrm merged commit 9c8c0a8 into main Mar 28, 2022
@meganrm meganrm deleted the fix/horizontal-scroll branch March 28, 2022 23:43
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.

3 participants