iD icon indicating copy to clipboard operation
iD copied to clipboard

Collapse sidebar narrow screen

Open AviralSingh-code opened this issue 1 year ago • 15 comments

Fixes #9815

AviralSingh-code avatar Sep 25 '23 14:09 AviralSingh-code

It looks like this PR contains a commit from #9914, so either that PR should be merged before this one, or this PR's branch should be rebased to start off from develop.

waldyrious avatar Sep 25 '23 17:09 waldyrious

Ya I will do the needful.

AviralSingh-code avatar Sep 25 '23 18:09 AviralSingh-code

Is it ok now ??

AviralSingh-code avatar Sep 26 '23 06:09 AviralSingh-code

@AviralSingh-code I managed to test this branch locally by following the instructions in the README. It seems to work as expected! :tada: however, I'd like to try out the behavior of the sidebar when objects are selected/deselected, but my locally running instance doesn't seem to load any geometry or background imagery. Is this the case for you as well? If not, do you have any hints as to how to get it to work locally?

Edit: I've asked in the OSM Discourse forum, let's see if anyone has hints there.

waldyrious avatar Sep 27 '23 14:09 waldyrious

Also, you probably should run npm run lint:fix to correct some code style issues with the way the code is currently formatted.

waldyrious avatar Sep 27 '23 14:09 waldyrious

Can you elaborate on the problem a bit more like the issue that you are facing while trying out the behavior of the of the sidebar when objects are selected/deselected because for me, everything seems fine on my local machine !! But maybe I am missing on to the details so if you could elaborate more on this.

AviralSingh-code avatar Sep 28 '23 08:09 AviralSingh-code

And yes there is one more question that the PR is showing the message "2 workflows awaiting approval", so do you have any idea on how to resolve that ?

AviralSingh-code avatar Sep 28 '23 08:09 AviralSingh-code

Can you elaborate on the problem a bit more like the issue that you are facing while trying out the behavior of the of the sidebar when objects are selected/deselected because for me, everything seems fine on my local machine !!

Cool, glad to hear it works well locally for you. In my case I can't see any OSM geometry when I run iD locally (with npm start), nor any background (satellite) imagery. All I see is black in the area of the editor where the map should appear. See the screenshot I added to my post in the Discourse forum linked above:

Do you see something different when you do npm start? Or perhaps you are using a different command?

And yes there is one more question that the PR is showing the message "2 workflows awaiting approval", so do you have any idea on how to resolve that ?

That's because you haven't had any contributions merged into this repository yet. So for now, you'll need a maintainer to explicitly trigger the checks. Once one of your PRs is merged, actions will start triggering automatically in further PRs.

waldyrious avatar Sep 28 '23 13:09 waldyrious

The first time I started the environment I had to do :

  1. npm install
  2. npm run all
  3. npm start Then whenever I made changes I always did :
  4. npm run all
  5. npm start

So I think maybe you could try to run npm run all before npm start. I am not sure whether that would help but no harm in trying !!

AviralSingh-code avatar Sep 30 '23 04:09 AviralSingh-code

Thanks for the suggestion, but running npm run all before npm start does not make a difference: I still see the iD interface, and it is functional, but nothing appears in the actual map area.

waldyrious avatar Sep 30 '23 14:09 waldyrious

What I feel is that you should try to reset your code with the upstream because I think that you might have altered the code that could have broken down the code because sometimes just to check the flow of the code I used to comment out the sidebar element or other elements and then the iD used to behave similar to what you are facing !!

AviralSingh-code avatar Oct 02 '23 18:10 AviralSingh-code

OMG, sorry, the issue was on my side — it looks like running iD locally doesn't work on Firefox. I can get the site to work normally if I use a Chromium-based browser, like Vivaldi or Brave. Apologies for the noise! :sweat_smile:

So, getting back on track: it seems like resizing the browser window to trigger the sidebar collapse makes the background imagery shift. Here's the current behavior in the develop branch:

Screencast from 02-10-2023 22:26:44.webm

And here is the behavior with this PR's branch:

Screencast from 02-10-2023 22:43:42.webm

Apologies for the frame skips — my graphics card isn't the best. But do try resizing the browser window yourself and you should notice the issue. IMO the background should remain static, i.e. "anchored" to the right edge of the window, rather than to the border of the sidebar.

waldyrious avatar Oct 02 '23 22:10 waldyrious

Do you have any suggestions on how should I try that ? Like what will be the best fix for this because I haven't added any code that affects the frame of the window but then also this shift is happening !!

AviralSingh-code avatar Oct 05 '23 13:10 AviralSingh-code

Yeah, indeed this behavior does not seem like it was introduced by your code, but it is there nevertheless. Unfortunately I don't know enough about this codebase to guide you towards what changes need to be done to avoid the issue, but hopefully once a maintainer of this project reviews this PR they will be able to point out the needed changes. In any case, that is a relatively minor issue since it's not expectable that people would be manually resizing their browsers frequently.

I did notice a separate issue, though: if you resize the browser to a small enough width that the sidebar is collapsed, then refreshing the page loads iD with the sidebar expanded. It seems like we need to perform the check not just in the resize event but also in the initial load event.

waldyrious avatar Oct 05 '23 18:10 waldyrious

Yes I will fix the initial load of the sidebar also !!

AviralSingh-code avatar Oct 06 '23 06:10 AviralSingh-code