Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a spinning icon #1137

Closed
jywarren opened this issue Oct 13, 2022 · 33 comments · Fixed by #1169
Closed

Add a spinning icon #1137

jywarren opened this issue Oct 13, 2022 · 33 comments · Fixed by #1169

Comments

@jywarren
Copy link
Member

After #1136, we'll be able to use icons.

image

In our MapKnitter Lite demo (https://publiclab.github.io/Leaflet.DistortableImage/examples/archive
), when you click "Place", it can take a few seconds to work. Let's think through some Javascript to show a spinning icon to show it's working, and to hide it again once it's done.

Let's work together in the comments here before we try to open a pull request: What are the steps to achieve this? What could some JavaScript code look like to make this work? What do we need to know, or what might go wrong that we should watch for?

Thank you! This is for folks who have completed a first-timers-only issue already.

@RealRichi3
Copy link
Contributor

One question, where would the spinning icon be displayed? Would it be on the side bar or the map?

And should the spinning icon cover have something like a grey overlay below it, and the overlay should cover and be above the map

@jywarren
Copy link
Member Author

jywarren commented Oct 13, 2022 via email

@RealRichi3
Copy link
Contributor

True, it'll be easier to implement

@RealRichi3
Copy link
Contributor

For this task, it think we'll need to know when the image has been placed on the map

First we need a way to know if the map has been placed or not

We can use a gif for the spinning icon, set the display property to hidden.
Then on click of the place on map button we can display the spinning icon.
The gif for the spinning icon will be visible as long as the image hasn't been placed on the map

@RealRichi3
Copy link
Contributor

RealRichi3 commented Oct 13, 2022

function isMapPlaced:
    // code to check if the map is placed

get spinning icon element from DOM
set spining icon element display to hidden

add an event listener to listen for click action on the `place on map`
       while not isMapPlaced:
             place the spinning icon above the clicked button
       set the spinning icon display to hidden

@RealRichi3
Copy link
Contributor

There might be better implementation for this tho, suggestions are welcomed

@jywarren
Copy link
Member Author

The button is created here, and we can add a click listener to it at that time, what do you think?

if (file.format === 'PNG' || file.format === 'JPEG'){
let imageRow = document.createElement('div');
let image = new Image(150, 150);
let placeButton = document.createElement('a');
placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button');
placeButton.innerHTML = 'Place on map';
image.src = `${url.replace('metadata', 'download')}/${file.name}`;
imageRow.classList.add('d-flex', 'justify-content-between', 'align-items-center', 'mb-4', 'pe-5');
imageRow.append(image, placeButton);
imageContainer.appendChild(imageRow);
imageCount++;
}
});
responseText.innerHTML = imageCount ? `${imageCount} image(s) fetched successfully.` : 'No images found in the link provided...'

@RealRichi3
Copy link
Contributor

perfect, we can add the click listener here, we also need to find a way to check if the image has been placed

@RealRichi3
Copy link
Contributor

I think knowing when the image has been placed would be the main issue, we can handle the rest with a simple logic

@RealRichi3
Copy link
Contributor

One question, how are the Images placed on the map, i mean what is the process it goes through. Do we just get the image, then position then add a new DOM element containing the image

@jywarren
Copy link
Member Author

One way to know is to track onload listener of the image. Another is to poll to see if the image has a width attribute. I think the onload listener would be best... let's also look if there is an event fired once the image finishes loaded.

@segun-codes
Copy link
Collaborator

Perhaps, we could consider SVG spinner instead of GIF spinning icon.

@jywarren
Copy link
Member Author

Its true, and we do have a loading spinner in our icons locally already I think. But I imagine we will be using a range of icons in this page so I think it's worth standardizing on FontAwesome. You can add the "spin" class to make it spin without using a gif!

@RealRichi3
Copy link
Contributor

Good idea, correct me if i'm wrong, so currently the plan is to

  • Add an click event listener for the place on map buttons
  • Add an onload event listener for the images
  • Onclick of the button we display the spinning icon
  • Then Onload of the image, the spinning icon disappears

If this is it, i would like to work on it then. Correct me if i missed anything

@RealRichi3
Copy link
Contributor

I was thinking about the event listener to check if the image has been added, i saw something about Mutation event, we can use this to check if a node or child element has been added to the parent div or element.

I had another idea tho,

  • Have a counter to store the current size of the images within the parent element in the DOM,
  • An async while loop to keep checking if the number of child elements in the parent element increases. It should be async we don't have to wait for it to complete.
  • If we notice any increment in the number of child elements we can then stop the spinning button and break out of the loop

The downside of this is that it might run forever incases where the image doesn't load. To account for this, we can set the async while loop to only run for a limited time, lets say, 30 seconds.

I think the first idea is better

@segun-codes
Copy link
Collaborator

That's yet another good idea. If we are going by the first idea as suggested, we should still consider implementing @jywarren's idea of waiting time. If no 'image dropped on map' event is fired after this waiting period elapses, then a message (e.g., "Please attempt to place the image again") can be displayed to the user. This can address network connectivity issue midway into a "place image on map" operation.

@segun-codes
Copy link
Collaborator

Hi @jywarren, the conversation here stimulated me into considering raising the two issues listed below. What do you think? Can I go ahead?

  1. provide drag and drop support for an image to be placed on a map: this is an alternative to using the 'place on map' button.
  2. provide hide and unhide functionality for the sidebar where we have the button 'place on map': currently, the sidebar sits on a reasonable percentage of the browser page, we can hide the sidebar when not needed (and unhide when required) so that user can have full benefit of the entire page for image distortion etc. I suspect such flexibility will offer better user experience.

@sainamercy
Copy link
Contributor

sainamercy commented Oct 14, 2022

The code for the click event is already there. the highlighted areas could be a simple fix:

document.addEventListener("click", (event) => {
if (event.target.classList.contains('place-button')) {
// render spinner
let imageURL = event.target.previousElementSibling.src;
let image = L.distortableImageOverlay(imageURL);
map.imgGroup.addLayer(image);
// image.addEventListener('load', ()=>{ // hide the spinner })
}
});

@sainamercy
Copy link
Contributor

sainamercy commented Oct 14, 2022

The button is created here, and we can add a click listener to it at that time, what do you think?

if (file.format === 'PNG' || file.format === 'JPEG'){
let imageRow = document.createElement('div');
let image = new Image(150, 150);
let placeButton = document.createElement('a');
placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button');
placeButton.innerHTML = 'Place on map';
image.src = `${url.replace('metadata', 'download')}/${file.name}`;
imageRow.classList.add('d-flex', 'justify-content-between', 'align-items-center', 'mb-4', 'pe-5');
imageRow.append(image, placeButton);
imageContainer.appendChild(imageRow);
imageCount++;
}
});
responseText.innerHTML = imageCount ? `${imageCount} image(s) fetched successfully.` : 'No images found in the link provided...'

How does one display part of a file like this

@RealRichi3
Copy link
Contributor

The code for the click event is already there. the highlighted areas could be a simple fix:

document.addEventListener("click", (event) => { if (event.target.classList.contains('place-button')) { // render spinner let imageURL = event.target.previousElementSibling.src; let image = L.distortableImageOverlay(imageURL); map.imgGroup.addLayer(image); // image.addEventListener('load', ()=>{ // hide the spinner }) } });

True, it applies the same logic we had in mind tho, I'll wait for @jywarren to approve before taking up the task

@7malikk
Copy link
Collaborator

7malikk commented Oct 14, 2022

map = L.map('map').setView([51.505, -0.09], 13);
map.attributionControl.setPosition('bottomleft');
map.addGoogleMutant();
map.whenReady(() => {
new bootstrap.Modal(welcomeModal).show();
});
};
const setupCollection = () => {
map.imgGroup = L.distortableCollection().addTo(map);

The button is created here, and we can add a click listener to it at that time, what do you think?

if (file.format === 'PNG' || file.format === 'JPEG'){
let imageRow = document.createElement('div');
let image = new Image(150, 150);
let placeButton = document.createElement('a');
placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button');
placeButton.innerHTML = 'Place on map';
image.src = `${url.replace('metadata', 'download')}/${file.name}`;
imageRow.classList.add('d-flex', 'justify-content-between', 'align-items-center', 'mb-4', 'pe-5');
imageRow.append(image, placeButton);
imageContainer.appendChild(imageRow);
imageCount++;
}
});
responseText.innerHTML = imageCount ? `${imageCount} image(s) fetched successfully.` : 'No images found in the link provided...'

How does one display part of a file like this

Hello @sainamercy, see how here

@sainamercy
Copy link
Contributor

Hi @7malikk. Thanks

@7malikk
Copy link
Collaborator

7malikk commented Oct 14, 2022

@segun-codes and @RealRichi3, this looks like an interesting issue I'd like to join you both in resolving it.

@jywarren
Copy link
Member Author

The code for the click event is already there. the highlighted areas could be a simple fix:

@sainamercy is correct, the click event is there!

I think @sainamercy has the right idea for inserting code in these lines:

#1137 (comment)

Maybe @sainamercy and @RealRichi3 would like to take this on together? @RealRichi3 if you open a PR with your attempt and @sainamercy can offer comments on it, that can count as a contribution for both of you. Just please note that it's collaborative in the PR description so we can count it for both.

@7malikk appreciate your help and if you can offer input that's great, but since @RealRichi3 and @sainamercy have already gotten a start, perhaps we can find another for you?

@segun-codes i like the hide sidebar option. What could it look like when minimized, so it can be re-opened? Maybe you and @7malikk could open a new issue for that and discuss the design ideas before starting a PR?

@7malikk
Copy link
Collaborator

7malikk commented Oct 15, 2022

@jywarren thats sounds great
@segun-codes how would you like to begin?

@RealRichi3
Copy link
Contributor

Alright @jywarren

@sainamercy
Copy link
Contributor

Hi @RealRichi3 I would be happy to collaborate with you

@segun-codes
Copy link
Collaborator

Alright @jywarren. @7malikk I will open an issue for this and then we can proceed to discuss design ideas.

@segun-codes
Copy link
Collaborator

@jywarren, what about the 'drag and drop' issue I suggested, do you want to react to it as well?

@7malikk
Copy link
Collaborator

7malikk commented Oct 15, 2022

Alright @jywarren. @7malikk I will open an issue for this and then we can proceed to discuss design ideas.

This sounds good @segun-codes
Ping me when you do

@jywarren
Copy link
Member Author

Regarding drag and drop, unfortunately there are some complications... I wrote a bit more in #998 (comment)

Thank you for your interest!!!

@segun-codes
Copy link
Collaborator

segun-codes commented Oct 15, 2022

Alright @jywarren. @7malikk I will open an issue for this and then we can proceed to discuss design ideas.

This sounds good @segun-codes Ping me when you do

Issue opened, tagged you and @jywarren. (here #1172)

@segun-codes
Copy link
Collaborator

segun-codes commented Oct 15, 2022

Regarding drag and drop, unfortunately there are some complications... I wrote a bit more in #998 (comment)

Thank you for your interest!!!

Oh! okay... thank you for clarifying @jywarren.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants