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

Connected pre-loaded images to v2 #436

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Connected pre-loaded images to v2 #436

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0086f20
Connected pre-loaded images to v2
stephaniequintana Jul 21, 2022
6f06a0e
included the function on src/ui/interface.js and dist/infragram.js
stephaniequintana Jul 27, 2022
1f7f118
simplified function & updated image size
stephaniequintana Aug 4, 2022
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
14 changes: 14 additions & 0 deletions dist/infragram.js
View file
Open in desktop

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

2 changes: 0 additions & 2 deletions dist/infragram2.js
View file
Open in desktop

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

21 changes: 18 additions & 3 deletions src/ui/interface.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ module.exports = function Interface(options) {
"#s_exp": "s",
"#v_exp": "v"
};

$("#overlay-slider").val(localStorage.getItem("overlaySize"));
console.log('grid ' + localStorage.getItem("overlaySize"));
setGrid($("#overlay-slider").val());

// TODO: broken:
// TODO: broken:
//urlHash.setUrlHashParameter(JSON.stringify(idNameMap));
src = urlHash.getUrlHashParameter('src');
if (src) {
Expand All @@ -68,6 +68,21 @@ module.exports = function Interface(options) {
return true;
});

$("#sample-image__select").click(function(e){
Copy link
Member

Choose a reason for hiding this comment

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

This looks good; it'll only run if there is an HTML DOM element with an id of sample-image__select, so this won't break v1. However, 2 thoughts -

  1. should we mark this as a v2 feature with a comment, so people know, and if we ever clean up all v1 code we can tell it apart
  2. should we otherwise explain this briefly in a comment, since it sounds a little mysterious?

Copy link
Member

Choose a reason for hiding this comment

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

What's cool is that this means ANY image could be used as a sample image, it just has to have the id sample-image__select. I suggest that we change that to a class, since strictly, ids should be used only once. Then someday someone may want to add, for example, a bunch of different examples in a kind of menu, they could use this. So the class could be maybe... .sample-image?

Copy link
Member

Choose a reason for hiding this comment

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

But perhaps that means we should then apply the class to just the individual images, rather than the whole container. Let me make a suggestion!

e.stopPropagation();
const img = (e.target.classList.contains('rfi')) ? document.getElementById('rfi') : document.getElementById('bfi');
Copy link
Member

Choose a reason for hiding this comment

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

Here's another super great line of code - i love that the idea is that you can add the "bfi" or "rfi" classes to switch between. Maybe we should explain this too in a comment!

Copy link
Member

Choose a reason for hiding this comment

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

Aha - i see now -- we want to click on the thumbnail, but use the big hidden image! I wonder - if we use relatively low res sample images, like, 1024x768 or less, could we simplify things by just skipping thumbnails? JPGs don't weigh that much, you know? Then the logic here can be much simpler - we just display the big images with style="width:200px;" or something like that!

Copy link
Member

Choose a reason for hiding this comment

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

So on this line, if we stop using separate thumbnails, we can maybe just skip using red-filter or blue-filter classes completely. Since we'd just select whatever image, and think about what kind of filter later?

fetch(img.src)
.then(res => res.blob())
.then(blob => {
const file = new File([blob], img.src, blob)
const fileInput = document.querySelector('input[type="file"]');
const dataTransfer = new DataTransfer();
dataTransfer.items.add(file);
Copy link
Member

Choose a reason for hiding this comment

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

Wow i really like this way of doing it. It literally selects the image, same as someone using it! That will mean that we don't have to support (or think about) 2 different means of adding images. Very cool.

fileInput.files = dataTransfer.files;
$(options.fileSelector).trigger("change");
})
});

$(options.fileSelector).change(function() {
$('.choose-prompt').hide();
$("#save-modal-btn").show();
Expand Down Expand Up @@ -130,7 +145,7 @@ module.exports = function Interface(options) {
localStorage.setItem("overlaySize", $("#overlay-slider").val());
$("#overlay-save-info").show().delay(2000).fadeOut();
});

$("[rel=tooltip]").tooltip()
$("[rel=popover]").popover()
return true;
Expand Down