Skip to content

Conversation

lakshz
Copy link
Contributor

@lakshz lakshz commented Dec 22, 2023

Fixes #2641

Before and after comparison
Screencast from 22-12-23 10:49:21 AM IST.webm

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Copy link

welcome bot commented Dec 22, 2023

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already.

@lakshz
Copy link
Contributor Author

lakshz commented Dec 22, 2023

@lindapaiste Could you please review this PR?

@lindapaiste
Copy link
Collaborator

I love the idea of managing the overlay in the PreviewFrame. Now that you are doing that for mobile you can do it for desktop too!

Delete these lines which render the overlay:

<div
className="preview-frame-overlay"
style={{ display: isOverlayVisible ? 'block' : 'none' }}
/>

And pass down the isOverlayVisible prop here:
<PreviewFrame cmController={cmRef.current} />

Also this is unrelated but while we're making changes here, we can delete these lines which are a relic of some legacy code that don't do anything (it's <div>{true}</div>). We're handling the accessible output in the EmbedFrame.

<div>
{((preferences.textOutput || preferences.gridOutput) &&
ide.isPlaying) ||
ide.isAccessibleOutputPlaying}
</div>

@lakshz
Copy link
Contributor Author

lakshz commented Dec 29, 2023

I love the idea of managing the overlay in the PreviewFrame. Now that you are doing that for mobile you can do it for desktop too!

Delete these lines which render the overlay:

<div
className="preview-frame-overlay"
style={{ display: isOverlayVisible ? 'block' : 'none' }}
/>

And pass down the isOverlayVisible prop here:

<PreviewFrame cmController={cmRef.current} />

Also this is unrelated but while we're making changes here, we can delete these lines which are a relic of some legacy code that don't do anything (it's <div>{true}</div>). We're handling the accessible output in the EmbedFrame.

<div>
{((preferences.textOutput || preferences.gridOutput) &&
ide.isPlaying) ||
ide.isAccessibleOutputPlaying}
</div>

@lindapaiste Got it. Changes done.

@lindapaiste lindapaiste merged commit a536a63 into processing:develop Dec 29, 2023
@raclim raclim added this to the MINOR Release for 2.11.0 milestone Jan 12, 2024
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.

Dragging the Console is sluggish when browser window is resized to mobile view

3 participants