p5.js-web-editor icon indicating copy to clipboard operation
p5.js-web-editor copied to clipboard

Find and Replace dialog box is not mobile-responsive

Open mrkirthi-24 opened this issue 2 years ago • 6 comments

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 avatar Jan 05 '24 10:01 mrkirthi-24

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

mrkirthi-24 avatar Jan 05 '24 10:01 mrkirthi-24

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

Yes please do, that's definitely a mistake.

lindapaiste avatar Jan 05 '24 17:01 lindapaiste

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

mrkirthi-24 avatar Jan 06 '24 17:01 mrkirthi-24

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?

lindapaiste avatar Jan 06 '24 18:01 lindapaiste

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

mrkirthi-24 avatar Jan 06 '24 19:01 mrkirthi-24

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.

lindapaiste avatar Feb 19 '24 21:02 lindapaiste