search

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

Find and Replace dialog box is not mobile-responsive #2839

Open mrkirthi-24 opened 10 months ago

mrkirthi-24 commented 10 months ago

p5.js version

v1.9.0

What is your operating system?

Mac OS

Web browser and version

Google Chrome 120.0.6099.129

Actual Behavior

The dialog box hides behind the editor and hence user is unable to click on the input fields and buttons.

https://github.com/processing/p5.js-web-editor/assets/53830546/bd1afd1f-d76f-4484-afb7-e87b78c7c124

Expected Behavior

The dialog box should appear above the editor.

Steps to reproduce

Steps:

  1. Open p5.js Web Editor in mobile screen
  2. In the Edit menu, click Replace option
  3. Find and Replace dialog box will appear
mrkirthi-24 commented 10 months ago

Hi @lindapaiste I would like to work on the issue if validated.

lindapaiste commented 10 months ago

Hi @lindapaiste I would like to work on the issue if validated.

Yes please do, that's definitely a mistake.

mrkirthi-24 commented 10 months ago

Hello, @lindapaiste should I leave space such that the editor is fully visible, or should the dialogue box cover the editor like in the example below?

Screenshot 2024-01-06 at 10 35 33 PM

lindapaiste commented 10 months ago

Hello, @lindapaiste should I leave space such that the editor is fully visible, or should the dialogue box cover the editor like in the example below?

Screenshot 2024-01-06 at 10 35 33 PM

Good question! I didn't even think about that! I was thinking that it was just a z-index issue. But what if the text that you are replacing is in the first 2 lines of the code which are covered up? I think that ideally we would shift the content down but that might be difficult to do given the hierarchy of the components.

The Editor would need to know that the find/replace modal is open. So we would need to pass around a lot of state. Well...actually...I think we can tell if the modal is open by looking at the internal state of the CodeMirror instance this._cm. But I would have to dig into the docs to figure out how to do that.

It would definitely be an improvement just to fix the z-index. So maybe we start with that, and revisit layout changes later?

mrkirthi-24 commented 10 months ago

It would definitely be an improvement just to fix the z-index. So maybe we start with that, and revisit layout changes later?

Great I will push the z-index changes for now

lindapaiste commented 8 months ago

The z-index issue was fixed by #3037.

I'm going to keep this issue open as we did not implement pushing down the editor to keep the top lines visible. But I'm editing the tags because the bug part is fixed, and what remains is a more complicated feature request involving CodeMirror code.