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.

Sign up for GitHub

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

Jump to bottom

load image in sidebar as thumbnail #1319

Merged
merged 9 commits into from
Jan 6, 2023
Merged

load image in sidebar as thumbnail #1319

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2933e48
load thumbnail in sidebar
7malikk Dec 22, 2022
c325e39
hot fix - render thumbnail in side bar
7malikk Dec 23, 2022
5e1d50d
add comments and refactor
7malikk Jan 4, 2023
2b89fc5
code-style changes
7malikk Jan 5, 2023
499f991
code-style changes
7malikk Jan 5, 2023
8d590f6
code-style changes
7malikk Jan 5, 2023
30590ae
render code refactor
7malikk Jan 5, 2023
20fe8f1
hot fix
7malikk Jan 5, 2023
c4281b7
Merge branch 'main' into patch-6.0
jywarren Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dist/leaflet.distortableimage.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

72 changes: 65 additions & 7 deletions examples/js/archive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ function extractKey() {

let imageCount = 0;
let fetchedFrom;
function renderImages(file, url) {
if (file.format === 'PNG' || file.format === 'JPEG') {

// the imgsBelow function is called when the images and thumbnails gotten from the data fetched are below 100
// it renders the images in the sidebar
const imgsBelow = (imgs, url) => {
imgs.forEach((file) => {
const imageRow = document.createElement('div');
const image = new Image(150, 150);
const placeButton = document.createElement('a');
Expand All @@ -73,13 +76,65 @@ function renderImages(file, url) {

placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button');
placeButton.innerHTML = 'Place on map';

image.setAttribute('data-original-image', `${url.replace('metadata', 'download')}/${file.name}`);
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++;
});
};

// this function is called when there are more than a 100 images and thumbnails in data fetched and also if there are not enough thumbnails per image
// it takes in three arguments thumbs, url and imgs
// using defualt function parameters and a logical OR, it is reuseable as a graceful failsafe for when there are no thumbnails for all images.
const getThumbs = (thumbs = [], url, imgs) => {
const display = thumbs || imgs;

display.forEach((file) => {
const imageRow = document.createElement('div');
const image = new Image(65, 65);
const placeButton = document.createElement('a');
fetchedFrom = document.createElement('p');
7malikk marked this conversation as resolved.
Show resolved Hide resolved
const fetchedFromUrl = document.createElement('a');
fetchedFromUrl.setAttribute('href', input.value);
fetchedFromUrl.setAttribute('target', '_blank');
fetchedFromUrl.innerHTML = 'this Internet Archive Collection';
fetchedFrom.appendChild(fetchedFromUrl);
const fileName = document.createElement('p');
fileName.innerHTML = file.name;
fileName.classList.add('m-0');
fileName.style.fontSize = '12px';

placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button', 'mt-1');
placeButton.innerHTML = 'Place';
placeButton.setAttribute('title', 'Place image on map');

image.setAttribute('data-original', `${url.replace('metadata', 'download')}/${thumbs ? file.original : file.name}`);
7malikk marked this conversation as resolved.
Show resolved Hide resolved
image.src = `${url.replace('metadata', 'download')}/${file.name}`;
imageRow.classList.add('col-4', 'd-flex', 'flex-column', 'p-2', 'align-items-center');
imageRow.append(image, placeButton, fileName);
imageContainer.appendChild(imageRow);
imageContainer.setAttribute('class', 'row');
imageCount++;
});
};


// renderImages function is to render the images fetched from IA
7malikk marked this conversation as resolved.
Show resolved Hide resolved
function renderImages(files, url, count) {
// thumbs variable is to extract the thumbnail files from the fetched data
const thumbs = files.filter(file => file.source === 'derivative');
// images variable is to extract the actual image files from the fetched data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, I think we can skip the comment if we use a very clear variable name - i'm a big fan of longer variable names that are very descriptive -- that way they're legible wherever in the codebase they're used. So what if we skipped the comments and named these variables thumbnail_images and original_images? (I would use camelCase but the lower case l and the capital I are hard to read...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks @jywarren, yea camelCase there is usually hard to read

const images = files.filter(file => file.format === 'PNG' || file.format === 'JPEG');

// this if check is to check the amount of image files and thumbnail files is below 100, afterwhich if true it renders the images in the sidebar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just thinking -- I feel like I want to have two functions named renderImages and renderThumbnails, and to use comments to show that the first is just plain images, and the second searches for thumbnails if they exist. The current renderImages is mostly just logic switching between those two, isn't it? And the thumbs filter could go into renderThumbnails because it's more specific to that task.

The logic (if loop) for which of those two functions we use could be more simply written:

// if over 100 images, show only thumbnails
if (count > 100) renderThumbnails(images, url);
else renderImages(images, url);

Then it's quite readable too - and this could go down on line 152, eliminating this intermediary function. Then we have 2 functions which are similarly named and act similarly, although one uses thumbnails instead. What do you think?

This would also keep all the filtering code together, which seems good too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I like the idea, it does make the code more readable.
I will work on the implementation, hopefully, it'll be a homerun from there.
Thank you @jywarren

if (count < 100) {
imgsBelow(images, url);
} else if (thumbs.length === images.length) { // <--- this check is to make sure there are thumbnails for each image, else it defaults to the last case
getThumbs(thumbs, url, images);
} else {
getThumbs(false, url, images);
}
}

Expand All @@ -89,9 +144,12 @@ function showImages(getUrl) {
axios.get(url)
.then((response) => {
if (response.data.files && response.data.files.length != 0) {
response.data.files.forEach((file) => {
renderImages(file, url);
});
count = response.data.files.filter((file)=> {
if (file.format === 'PNG' || file.format === 'JPEG' || file.format.includes('Thumb') ) {
return file;
}
}).length;
renderImages(response.data.files, url, count);
responseText.innerHTML = imageCount ? `${imageCount} image(s) fetched successfully from ${fetchedFrom.innerHTML}.` : 'No images found in the link provided...';
} else {
responseText.innerHTML = 'No images found in the link provided...';
Expand Down Expand Up @@ -124,7 +182,7 @@ tileMap.addEventListener('click', (event) => {

document.addEventListener('click', (event) => {
if (event.target.classList.contains('place-button')) {
const imageURL = event.target.previousElementSibling.src;
const imageURL = event.target.previousElementSibling.dataset.original;
const image = L.distortableImageOverlay(imageURL);
map.imgGroup.addLayer(image);
}
Expand Down