infragram icon indicating copy to clipboard operation
infragram copied to clipboard

Connected pre-loaded images to v2

Open stephaniequintana opened this issue 2 years ago • 8 comments

Allows the user to select a pre-loaded image to explore the tool with.

Fixes the "Connect Sample Images" from Planning issue #0415

Caveats:

  • the index2.html includes the code from #0435 which corrects keyboard-accessibility of the <input> element (see question in comments)
  • the function for uploading the selected sample image has ONLY been added to the dist/inframgram2.js file
    • it has not beeen added to the src/ui/interface.js file
    • Q: how do we proceed with this? Create an interface2.js file for this version? Please advise.
  • [x] PR is descriptively titled 📑 and links the original issue above 🔗
  • [x] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
  • [x] code is in uniquely-named feature branch and has no merge conflicts 📁
  • [x] screenshots/GIFs are attached 📎 in case of UI updation
  • [x] ask @publiclab/reviewers for help, in a comment below

stephaniequintana avatar Jul 21 '22 18:07 stephaniequintana

gitpod-io[bot] avatar Jul 21 '22 18:07 gitpod-io[bot]

@jywarren, @TildaDares, @cesswairimu

Clarifications/Discussion requested:

  • In addition to not having added the function of this PR to the interface.js file (or having created an interface2.js file), I realize that the functions altered in the-previously-merged-PR #0431 (namely changing multiple occurances of $('#preset-modal').modal('show'); to $('#preset-modal').offcanvas('show');) were also only added to the infragram2.js file and NOT to the interface.js file. How might we best handle these functions that only pertain to this version?
  • It does not seem correct to have added the code from another PR into this PR...Is it correct that NOT including it in this PR would just create a simple merge-conflict? I believe I've confused myself into thinking that by not including that code into this PR then if that PR were merged first, it would would be overwritten when this PR is merged. I realize this question is poorly articulated, no need to reply if it's not clear :/

I hope this finds you all well :)

stephaniequintana avatar Jul 21 '22 18:07 stephaniequintana

Hi, no issue, it's confusing! Merging will incorporate only whats shown in the changed files here, and the merge action always prioritizes the active PR's changes. You should just before merging be able to see in the diff view the green + lines that will be the final state after the merge is conducted. Once a merge is completed, the diff view and changed files in all other PRs will be modified to show the new post-merge changes taking into account the action that just happened. Does that make sense?

It's OK to include changes in both - it's very likely they'll then be dropped from the diff of the other, as that change will already have occurred, OR it may say there's a conflict and it can't be merged automatically.

jywarren avatar Jul 22 '22 21:07 jywarren

functions altered in the-previously-merged-PR https://github.com/publiclab/infragram/pull/431 (namely changing multiple occurances of $('#preset-modal').modal('show'); to $('#preset-modal').offcanvas('show');) were also only added to the infragram2.js file and NOT to the interface.js file. How might we best handle these functions that only pertain to this version?

Should we try this idea of detecting what version we're running? I'll make a code suggestion so the flag will exist!

jywarren avatar Jul 22 '22 21:07 jywarren

I started the flag implementation in https://github.com/publiclab/infragram/pull/438 - i will try to generate the dist files too -- but if that makes sense we can merge it and you can begin using if (options.version == 1) {} in your code!

jywarren avatar Jul 22 '22 21:07 jywarren

@jywarren, I included the function in the $(document).ready(function(){...} of the src/ui/interface.js.

Please ensure that the function was placed in the correct location.

I've also include it in the dist/infragram.js file in preparation of dist/infragram2.js becoming obsolete. The code does not break the original version and thus I did not utilize the version flag (implemented in #0438).

You can view this in action at https://stephaniequintana.github.io/infragram/index2.html

stephaniequintana avatar Jul 27 '22 13:07 stephaniequintana

@jywarren,

Great suggestions! Thank you 🙏

I originally put the eventListener on the entire div in case a user clicks the text (instead of the image), but this came with having to add many classes to many elements. This way is MUCH cleaner and the function itself is more clear. for your quick reference, it is now:

$(".sample-image").click(function(e){
          e.stopPropagation();
          const img = (e.target);
          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);
              fileInput.files = dataTransfer.files;
              $(options.fileSelector).trigger("change");
            })
        });

I also deleted the thumbnails and replaced the images with smaller versions (width: 1024).

I am working on a better design for the welcome-container as a whole and will include this in a separate UI PR.

stephaniequintana avatar Aug 04 '22 21:08 stephaniequintana

Sounds super!!!

On Thu, Aug 4, 2022, 5:46 PM Stephanie Quintana @.***> wrote:

@jywarren https://github.com/jywarren,

Great suggestions! Thank you 🙏

I originally put the eventListener on the entire div in case a user clicks the text (instead of the image), but this came with having to add many classes to many elements. This way is MUCH cleaner and the function itself is more clear. for your quick reference, it is now:

$(".sample-image").click(function(e){

      e.stopPropagation();

      const img = (e.target);

      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);

          fileInput.files = dataTransfer.files;

          $(options.fileSelector).trigger("change");

        })

    });

I also deleted the thumbnails and replaced the images with smaller versions (width: 1024).

I am working on a better design for the welcome-container as a whole and will include this in a separate UI PR.

— Reply to this email directly, view it on GitHub https://github.com/publiclab/infragram/pull/436#issuecomment-1205801491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J2YWVOA6CLSXRUKZLTVXQ24JANCNFSM54IQ4XXQ . You are receiving this because you were mentioned.Message ID: @.***>

jywarren avatar Aug 04 '22 22:08 jywarren