processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.39k stars 1.33k forks source link

Clicking on down `chevron` beside clear does not shifts the console to bottom in `mWeb(Safari)` && `mWeb(chrome)` #2631

Closed PiyushChandra17 closed 9 months ago

PiyushChandra17 commented 11 months ago

p5.js version

2.9.3

What is your operating system?

Mac OS

Web browser and version

No response

Actual Behavior

Clicking on down chevron beside clear does not shifts the console to bottom in mWeb(Safari) && mWeb(chrome)

https://github.com/processing/p5.js-web-editor/assets/47579287/f6e08040-04b7-4f78-a769-9b763f42784b

Expected Behavior

Clicking on down chevron beside clear should shift the console to bottom in mWeb(Safari) && mWeb(chrome)

Steps to reproduce

Steps:

  1. Open the editor web app either in iOS(safari) or android(chrome)
  2. Click on down chevron beside clear
  3. Notice it doesn't shifts to bottom after taping on chevron
lindapaiste commented 11 months ago

Good catch. This is a bug and should be fixed. You can look into how we are doing this on the desktop version and apply that to mobile.

PiyushChandra17 commented 11 months ago

@lindapaiste Sure, I am working on it, will raise a PR soon. Also can you please assign this to me. Thanks!

lindapaiste commented 11 months ago

@PiyushChandra17 The issue is missing props on the SplitPane component which is used to resize the console area. On the desktop version this is a controlled component where we store the size in state and pass it down. The critical line is this one, where we shrink the size to 29 (just the size of the top bar) when the console is collapsed. https://github.com/processing/p5.js-web-editor/blob/fcbc05e2e65d934ea5416599b60f71592aedecf7/client/modules/IDE/pages/IDEView.jsx#L174

Right now on mobile it is an uncontrolled component but it should be controlled the same way using the same state.

Compare the desktop part: https://github.com/processing/p5.js-web-editor/blob/fcbc05e2e65d934ea5416599b60f71592aedecf7/client/modules/IDE/pages/IDEView.jsx#L171-L179

To the mobile part: https://github.com/processing/p5.js-web-editor/blob/fcbc05e2e65d934ea5416599b60f71592aedecf7/client/modules/IDE/pages/IDEView.jsx#L213-L218

We need to copy the rest of those props, so that it looks like this:

<SplitPane
  style={{ position: 'static' }}
  split="horizontal"
  primary="second"
  size={ide.consoleIsExpanded ? consoleSize : 29}
  minSize={29}
  onChange={(size) => setConsoleSize(size)}
  allowResize={ide.consoleIsExpanded}
  className="editor-preview-subpanel"
>

I would close your existing PR and put in a new one since this is very different.

lindapaiste commented 11 months ago

Once the console is closed the chevron button becomes very hard to click. This is a separate but related issue. You don't have to fix it but you can if you want to. image

I went back and looked at the original designs and we had the stop button above the console, rather than in the far bottom right. image

We need to pass down the consoleSize as a prop to the FloatingActionButton and override the bottom position to include the additional offset. But only for the "stop" button and not the "play" button. Maybe something like this in the IDEView:

<FloatingActionButton
  syncFileContent={syncFileContent}
  offsetBottom={ide.isPlaying ? consoleSize : 0}
/>

And then make changes to the FloatingActionButton component to use the prop.