Move layer visibility toggle to the left of the layer title and icon

[mfisher87]

Description

Resolves #244

Before:

After:

Keeping the button on the right raises questions about how to handle when the button overlaps with the text. After playing with it for a bit, I felt that having the button on the left is ideal. Especially as we move towards implementing #486, I feel having the button on the right misses an opportunity to convey information about grouping through indentation of the button. With the button on the right, the eye has to do more scanning from left-to-right.

Checklist

• PR has a descriptive title and content.
• PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
• PR has one of the labels: documentation, bug, enhancement, feature, maintenance
• Checks are passing.
  Failing lint checks can be resolved with:
  • pre-commit run --all-files
  • jlpm run lint

---

📚 Documentation preview: https://jupytergis--487.org.readthedocs.build/en/487/
💡 JupyterLite preview: https://jupytergis--487.org.readthedocs.build/en/487/lite

[github-actions]

👈 Launch a Binder on branch mfisher87/jupytergis/move-visibility-toggle

[github-actions]

Integration tests report: appsharing.space

[mfisher87]

Boo to that linting rule 😆 whitespace can be an incredibly valuable tool for readability. Anyone feel strongly about changing it?

[mfisher87]

please update snapshots

[arjxn-py]

The snapshots didn't update, don't know why - I'd just ask the bot to please update snapshots again

[arjxn-py]

Oh it fails again - https://github.com/geojupyter/jupytergis/actions/runs/13523809762/job/37789149195?pr=487#step:11:3347

I haven't looked in too deep yet

[martinRenou]

@arjxn-py do we still need to port jupytercad/JupyterCAD#694 ?

[arjxn-py]

@arjxn-py do we still need to port jupytercad/JupyterCAD#694 ?

Ahh could be, let's try.
EDIT: ./jupytergis/dist/jupytergis*.whl is already being installed ./jupytergis_app/dist/jupytergis*.whl is not there

[arjxn-py]

ping @martinRenou

[martinRenou]

Do you have the logs to the failing run of the bot?

[arjxn-py]

Oh it fails again - geojupyter/jupytergis/actions/runs/13523809762/job/37789149195?pr=487#step:11:3347

Yes, here

[martinRenou]

From the logs of the visual regression tests run https://github.com/geojupyter/jupytergis/actions/runs/13475445612/job/37654119187?pr=487, it looks to me the tests need updating, not the snapshots

[mfisher87]

Thanks for y'all's help figuring that out! ❤️

I'll try and figure out what I broke in the UI tests :) Wish me luck! ☘️

[martinRenou]

Welcome to the hell of ui-testing :D

It seems at least one test is failing here

jupytergis/ui-tests/tests/left-panel.spec.ts

Line 122 in a6d7ef5

await expect(layerIcons.first()).toHaveAttribute(

It looks like the test expects the first icon is the layer icon, and it gets your visibility icon now instead.

[mfisher87]

Any tips for running UI tests locally? I added some instructions 00ee3d1 (EDIT: ReadTheDocs appears to be having a hiccup, I'm getting 500 errors trying to access the web UI) as I was trying to figure it out, but after overcoming those hurdles, each test is failing after 60 seconds.

  1) tests/annotations.spec.ts:22:7 › #annotations › Should be able to add an annotation ───────────
                                                                                               
    Test timeout of 60000ms exceeded while running "beforeEach" hook.                                                                                                                          
                                                                                                                                                                                               
      12 |     );
      13 |   });                                                                                                                                                                               
    > 14 |   test.beforeEach(async ({ page }) => {
         |        ^                                                                            
      15 |     await page.filebrowser.open('testDir/annotation-test.jGIS');
      16 |   }); 
      17 |      
        at /home/shared/jupytergis/ui-tests/tests/annotations.spec.ts:14:8
                                               
    TypeError: Cannot read properties of null (reading 'activity')      
                                               
      17 |
      18 |   test.afterEach(async ({ page }) => {                         
    > 19 |     await page.activity.closeAll();
         |                ^                                                                    
      20 |   });
      21 |
      22 |   test('Should be able to add an annotation', async ({ page }) => {
        at /home/shared/jupytergis/ui-tests/tests/annotations.spec.ts:19:16
                                               
    Error: page.evaluate: Target page, context or browser has been closed
        at /home/shared/jupytergis/ui-tests/node_modules/@jupyterlab/galata/src/jupyterlabpage.ts:717:17
        at Timeout.check (/home/shared/jupytergis/ui-tests/node_modules/@jupyterlab/galata/src/utils.ts:183:33)

Am I missing something? Along the way, it required me to install a system dependency sudo apt-get install libavif13 which skeeves me out. Should we set up a docker container or something for predictably running these tests locally?

[mfisher87]

@arjxn-py got me on track in Zulip. I updated the docs with what I learned!

[martinRenou]

Thanks! And thanks for adding some docs

[martinRenou]

I would put a big warning here. Running the tests locally isn't reliable and I'd not recommend it, things like font rendering and WebGL depend a lot on the browser and GPU you're using.

So I'd recommend not taking the local runs for granted, and not updating the snapshots from a local run.

I always wondered if tools like act made things better locally, though I never tried really, and I guess all GPU related things are still dependent on the running machine.

[mfisher87]

things like font rendering and WebGL depend a lot on the browser and GPU you're using

The GPU, too!? Wow. I thought browser consistency was handled by playwright downloading the same version of Chromium for every test run. Oof. Local runs without snapshot tests should be OK, though, right? Maybe there's a way we can exclude them and document that for local running. Having the Playwright Inspector running locally really helped with debugging the test.

[mfisher87]

5c97cd0 How's that?

[martinRenou]

Oh neat! I did not know you could tell playwright to ignore snapshots!

[mfisher87]

The linter was being a jerk on the previous commit. It was "autofixing" by escaping the asterisks I was using to define bold text. I was only able to fix it by getting the opening and closing asterisks on the same line.

[martinRenou]

Thanks!