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

Re: Sim missing at exactly 991px width from MicroBit repo #9621

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

emoltz
Copy link
Contributor

@emoltz emoltz commented Jul 29, 2023

This is concerning issue #5328 in the Microbit repo (microsoft/pxt-microbit#5328). The issue was that the simulator was not showing up when the window width was exactly 991px wide. After a lot of digging I was able to pinpoint that the issue was stemming from the isTabletSize() function within the browserutils.ts file.

return window?.innerWidth = pxt.BREAKPOINT_TABLET;

to

return window?.innerWidth <= pxt.BREAKPOINT_TABLET;

`        return window?.innerWidth = pxt.BREAKPOINT_TABLET;
`

to

`        return window?.innerWidth <= pxt.BREAKPOINT_TABLET;
`
@emoltz emoltz changed the title Re: Sim missing at exactly 991px width #5328 Re: Sim missing at exactly 991px width from MicroBit repo Jul 29, 2023
Copy link
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Fix looks good to me, but could you also remove the = from isComputerSize just below so they don't both return true for innerWidth == 991?

@@ -166,7 +166,7 @@ namespace pxt.BrowserUtils {
}

export function isTabletSize(): boolean {
return window?.innerWidth < pxt.BREAKPOINT_TABLET;
return window?.innerWidth <= pxt.BREAKPOINT_TABLET;
}

export function isComputerSize(): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

this function already has it >=, so we need to fix that so we never end up with both isTabletSize() === true & isComputerSize() === true at the same time

@emoltz
Copy link
Contributor Author

emoltz commented Aug 1, 2023

Certainly!

Copy link
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

Perfect, thank you so much! Will let updated build go through then merge, I'll probably integrate it into a hotfix release for micro:bit ~end of week as I have a few more bugs I'm looking at sneaking in fixes for

@emoltz
Copy link
Contributor Author

emoltz commented Aug 1, 2023

Sounds great. I will look at other bugs to fix.

@jwunderl
Copy link
Member

jwunderl commented Aug 1, 2023

(if you want a related one, I believe there is a minor spacing issue still with the download button having extra left margin when in a tutorial 991px width -- this fixed most of the issues related to tutorials at the breakpoint though! This one should be in the .less though / probably a bit annoying to nail down)

@emoltz
Copy link
Contributor Author

emoltz commented Aug 1, 2023

I'll take a look. I'm in Colombia at the moment with limited internet but will fix once I am back in the states.

@jwunderl jwunderl merged commit d3861d3 into microsoft:master Aug 1, 2023
5 checks passed
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.

2 participants