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

Update index2.html #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update index2.html #1

wants to merge 1 commit into from

Conversation

Forchapeatl
Copy link
Collaborator

@Forchapeatl Forchapeatl commented Jul 1, 2022

Screenshot from 2022-07-01 03-42-12

Hello @stephaniequintana . I hope you are doing well I made some changes but there still exist limitations which affect the current logic functionality. Please accept these kind remarks. Thank you!

Limitations

Above all have a great day!

@stephaniequintana
Copy link
Owner

Thank you so much, @Forchapeatl, you are an absolute God-send!

I've made quite a few changes towards getting the functionality going.

I did change the id="quickStartGuide" to id="preset-modal" in the copy that I am working on, but at this time, the functions themselves call for "modal" example: $('#preset-modal').modal('show');. These will need to be changed in the js/scr files to offcanvas, example: $('#preset-modal').offcanvas('show');
Also to this point, there is a separate offcanvas component for the mobile view. Currently, I have added id="preset-modalMobile" and will need to add that functionality to the js files... I wonder if it would be better to name them more appropriately, but I didn't want to veer too far off from the js files.

I'm still trying to figure how the <select> and <form> elements can be fit into the current styling. I'm assuming that they need to remain as <select> and <form> elements so I'm working toward this angle.

I've mostly been concentrating on getting an image to load and nothing I do seems to be working. The file itself is uploading, but the image is not connecting to the canvas. Part of me wants to revert to simply re-styling the current site as-is, but I like this UI and am going to stick with it for now. I'll reach out to Jeff for help soon if I can't make any headway.

My eyes are tired and I haven't had chance to see exactly what all you have added in, but off the bat I see several classed that I have not included. THANK YOU, THANK YOU, THANK YOU!

I am going to begin pushing my saved changes more often so the most current copy I'm working on will be more readily available to you. Also, I'll incorporate what you have added in once I get a bit of rest.

Thank you again, Forcha ❤️ I hope you are doing well 😄

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 1, 2022

You 're welcome @stephaniequintana this is file caused the problems
<script src="node_modules/bootstrap5/dist/js/bootstrap.bundle.min.js">
I used the formmer file without the 5 things should work properly now. I love this new UI. Congratulations!

@stephaniequintana
Copy link
Owner

Hi, @Forchapeatl && happy Monday! 🚀 🌟 - I hope your doing well.

I've been down quite a few rabbit holes the last couple of days, but I've finally updated a more functional copy of the working draft. I incorporated many of your changes - THANK YOU again for the help 💟 but I did not merge this PR bc I already had so many changes of my own that I didn't want to overwrite. I may have missed a few things, if so, please don't hesitate to let me know - (that goes for anything you would like to change or for us to work on) and we'll get them in the code.

This copy is not completely functional yet - I'm still working on some css issues and a few of the functions are janky. There are probably others that I'm not even aware aren't working, 😬 😰 and others that still need attention in the current https://infragram.org/sandbox/index.html ...

please let me know what you think, especially of specific things I/we need to focus on and perhaps where you think we need to be before we can both begin working directly with these files.

fyi, I did create/add an infragram2.js file so that I could make changes that are only pertinent to this version (for now I've only fixed/added functionality for the quick-start offcanvas component). I did NOT add anything to the src files as I'm still uncertain of how we are going to move forward with those files for this version.

I'm going to continue making adjustments (css, overlay-container, ...) before creating a PR, but I would like to have this merged soon so that I can begin focussing on smaller, more specific tasks on the daily.

Also, I'd like to be able to include the your pending PRs - GREAT work, btw! 🔥 KUDOS to you!
What do you think the best way to do that is? I know that @jywarren would like both versions to have the functionality that you are adding, but I'm not sure of the best path on this one. Thoughts?

So, I'm sure I've forgotten much, but will leave you with this for now.
I hope you're having a great day 😄

p.s. this most current branch is v2_functions_connect and it can be viewed at https://stephaniequintana.github.io/infragram/index2.html

@Forchapeatl
Copy link
Collaborator Author

@stephaniequintana , the UI looks amazing!. Great Job!

I did not merge this PR bc I already had so many changes of my own that I didn't want to overwrite

No problem . I prefer this method . Your code base is readable. I will just alert my changes through Pull requests.

PLease have a look at this #6

@jywarren
Copy link
Collaborator

jywarren commented Jul 5, 2022

Hi, this is looking super! Great work everyone. So, as to the order, which PRs should we merge first? The other option is that we can pull some branches into other branches and create a combined PR. Or, we could open a PR from Forchapeatl-patch-3 against Stephanie's branch, and merging it will combine the PRs. Let me know the ideal order and I can advise!!

@stephaniequintana
Copy link
Owner

stephaniequintana commented Jul 5, 2022

Ok, thank you @jywarren - we did merge @Forchapeatl's work on Local video processing , Resolution Change and Canvas Recording last night. The current gh-pages branch reflects this and it can be viewed at https://stephaniequintana.github.io/infragram/index2.html

We would like to move the controls we added to a better area (as of rn they are in the bottom right corner of the screen) and I am currently working on positioning a few elements as well as the responsiveness on the mobile layout. I am hoping to open a PR later today so that we can both begin merging directly with the PL Infragram repo.

We would love and appreciate any feedback you have on it.

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 5, 2022

@jywarren and @stephaniequintana , please spare me some few minutes. I forgot to include the updated src files.

@stephaniequintana
Copy link
Owner

@jywarren - no rush, please take your time. I have lots to do in wait...

@Forchapeatl
Copy link
Collaborator Author

@jywarren - no rush, please take your time. I have lots to do in wait...

you mean't @Forchapeatl . Hahaha

@jywarren
Copy link
Collaborator

jywarren commented Jul 5, 2022

Ok no rush at all let me know when you'd like something merged! Esp in the chat if it's time sensitive. Great collaboration!!!

@stephaniequintana
Copy link
Owner

@Forchapeatl, I've just pushed my latest changes and updated the site...

I did move the video controls to the top menu (their display is toggled when you click the camera btn) and the pause/seekbar are now showing. I changed the styling just to fit this positioning, we can update these and/or move them as we progress. I also stopped the quick-start menu from opening when the btn is clicked - mostly out of annoyance - but I'd like to discuss our intended flow so that we can better guide the user.

For instance, do we want/need to push the user to choose a resolution, then an analysis mode and then enable the video buttons? Do we need the quick-start menu to open at all in video mode?

I also adjusted the mobile layout and will tend to more specific aspects of the design as I move forward.

Please let me know if you would like to update this branch with any changes or additions before I open a PR for it to be merged with PL.

I hope the day is treating you well 😄

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 6, 2022

Hello @stephaniequintana You are the goddes of UI/UX . The UI is Excellent !!
Please
image
this image controls both image / video uploads. Please , the video controls should be activated by this button not the camera button (icon). Great job so far. Sorry for the inconvenience It seems I switched the button locations at my former PR.

For instance, do we want/need to push the user to choose a resolution,

The default resolution is set to 800px by 600px. I think the best option is to leave it to the welcome modal (Navigation guide) to push the user.

@stephaniequintana
Copy link
Owner

stephaniequintana commented Jul 6, 2022

@Forchapeatl, You are too kind! (but I'll take the compliment) - Thank you 🙏 ❤️

No worries on the video controls, that's an "easy" fix. Two things:

  • Are we not loading the video controls when the webcam is activated also? I do need to get a better idea of the flow; Perhaps I need to study up more on the webcam itself...
  • I say "easy" fix as we merely have to change the element that the onClick function is triggered. We should actually only do this when a video is uploaded and not when an image is uploaded. Is this correct? If so, I need guidance with writing the function...currently it's:
    function toggleToolbar() {
      $('.mediaSelect').toggle();
      $('.videoControls').toggle();
    }
// and I will need to add an if statement, but I do not 
// know the specific syntax of targeting the file type...
   ?? if(#file-sel.value ==  video/*)  ??

I'm sure I can find this, but if you know and can share (HELP!!) I'm certain that would save me a lot of time 😉


The default resolution is set to 800px by 600px. I think the best option is to leave it to the welcome modal (Navigation guide) to push the user.

Ok, I will leave the quick-start guide as is (currently not opening when the webcam is activated) and I will include guidance in the welcome/navigation tour that is specific to the webcam itself.

Lastly, I wanted to ask if you got your gh-pages figured out? I learned that I was doing it ALL WRONG 😱😄 but I think I've got it down now. It might be easiest to pull from my gh-pages branch then merge/push whatever branch you want published into that. Let me know if I can help 👍

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 7, 2022

We should actually only do this when a video is uploaded and not when an image is uploaded. Is this correct?

Sounds good. I will implement this by July 22nd. For now I will try to implement the canvas panning and zooming.

I think it's best for me to Fork your repository ( in regards to gh-pages). Thank you

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 17, 2022

Hello @stephaniequintana hope you are doing well. I am unable to open an issue. I will just keep it here . So we don't forget.
Full-screen assumes a smaller div size after exit

e9088fb2-343d-44d4-bb8a-3344eab27888.mp4

@stephaniequintana
Copy link
Owner

Hi, @Forchapeatl - I hope this finds you well &&& my apologies for being out-of-touch for so long; I frustratingly caught Covid and am just recently functioning normally again.

I do have a few questions about your recent work - GREAT JOB, btw 🚀 🌟 I love that you're able to write such code!

So - please correct any of my misunderstandings or omissions - in PR #421 you recently added in functionality for processing an uploaded video. This is in addition to your earlier code in that same PR which processes the video from connecting to a webcam. Is this correct?

I want to clarify for myself so that I know what needs to be included/merged and perhaps discuss the best approach for getting all of your work into the most recent PL Infragram files. Also, I know that I undid some of your work when I moved the #custome-seekbar (my apologies!!) so I definitely want to remedy that 😄

I'm noticing that PR #421 has an index3.html file which I believe holds the code for the current sandbox page, but I see in your comment here that there is also code for the version2 (index2.html), but I don't see that as a listed file that has changed in the PR. I may have missed it. Because the code for index3.html will need to be added/changes/updated to the index.html file and some of the functions and index2.html code has already been merged via PR #431. I wonder if it might be easiest for you to open separate PRs for the code that has not yet been merged? What are your thoughts? Of course, this is just a suggestion, if you already have a solution, please disregard.


Next - from your most recent comment above,

Full-screen assumes a smaller div size after exit

I cannot quite discern from the video what is happening. I assume this is from recent work you've done with entering/exiting Fullscreen(?) I'm curious bc I cannot tell from the clip what exactly is being clicked. It is interesting & I'd love to know what's causing it and hopefully help in fixing it... I'm curious about the issue as a whole and would like to address

I am unable to open an issue. I will just keep it here . So we don't forget.

Are you able to open the issue or PR on publiclab/infragram with your code or are you not quite ready for that? If you would like to invite me as a collaborator on your fork it might be easier to share ideas, or let me know which branch the code is in, maybe I can see from there.. Please let me know if/how I can help address the Fullscreen issue.


I apologize for bombarding you a lot at once. The UI took more than twice as long as I had hoped and planned for and I feel like that put us both behind. I want you to know that I am back in this and will finish strong. I did recently open #435 and #436 and will be focussing on smaller tasks/corrections for the sandbox page while I plan for the homepage and the welcome/navigation tour.

Ahhh - && I see that you just opened #437 - KUDOS!! You're on 🔥❗

Have a great day/night!

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 23, 2022

Hello @stephaniequintana the existing PRs correspond to their respective functionalities (topics) along side limitations. I need to address those limitations separately. It's just my own way of tracking the functionality structure and file changes. I made the PR to your repository in so as to avoid the trouble of gh pahes. I am sorry for the mess on your repository. I will clean up my work and close unnecessary PRs. Please don't merge my recent works. In about a weeks I will submit a clean and ready PR. In the main time please move forward with your changes .Thank you :) .

Full-screen assumes a smaller div size after exit

This should be as a result of accepting multiple resolutions . I will work on it

@stephaniequintana
Copy link
Owner

@Forchapeatl, sounds good - & no worries about cleaning up my repo... I am always ensuring that my main branch is the same copy as that of publiclab/infragram and am keeping any current work on separate branches, so you adding in branches/features is not a problem. In fact, I like that I have access to them to see how they will all fit together! Hope you're doing well 😄

@stephaniequintana
Copy link
Owner

stephaniequintana commented Jul 27, 2022

@Forchapeatl, I recently updated Gitub pages to reflect my work on connecting the pre-loaded images... I forgot that your work was published on my Github Pages branch so when I rebased it to my working branch I overwrote the commits of your latest work.

This means that the link that you have used to publish your work in #0437 - https://stephaniequintana.github.io/infragram/index3.html# - is no longer available. My apologies.

We can remedy this. I could make a separate branch of my latest work and merge your working branches into that and we could publish that combined work.

Or, if you would rather, from your forked repo you can
git checkout gh-pages and then pull from my gh-pages branch,
git pull https://github.com/stephaniequintana/infragram.git gh-pages
and then point to your working branch, git rebase the-branch-you-want-published

Please let me know if you want me to create a merged branch so that your PRs can have a working reference.

@Forchapeatl
Copy link
Collaborator Author

Forchapeatl commented Jul 27, 2022

@stephaniequintana , All is well, I will use the fork. No problem. I get 404 from this link https://stephaniequintana.github.io/infragram/index3.html# please is this the updated branch ?

@stephaniequintana
Copy link
Owner

stephaniequintana commented Jul 28, 2022

@Forchapeatl, the latest updated branch is https://stephaniequintana.github.io/infragram/index2.html#

Please feel free to reach out to me if you run into any problems - I know better than anyone that publishing to Github Pages is not always a smooth process.

Hope you're well and having a great day :)

@stephaniequintana
Copy link
Owner

@Forchapeatl - I've just noticed that you have pushed some commits onto my main branch:

Screen Shot 2022-07-27 at 8 43 06 PM

I need to know if you have this work saved somewhere other than on my main branch. I know its not completely necessary, but my process it to always have my main branch in-line and current to what is on the publiclab main branch.

Screen Shot 2022-07-27 at 8 49 41 PM

I am very much in the habit of pulling from publiclab/infragram and then pushing to stephaniequintana/infragram so that when I rebase my working branches to main they are pointing to the most up-to-date commits.

Doing this right now will overwrite the work that you have pushed to my main branch. I very much want to avoid overwriting your work so I need to know if you have saved that work somewhere else. It appears that it is not saved on your own main branch as those commits are different than the ones on mine:

Screen Shot 2022-07-27 at 9 02 50 PM

For now, I can simply maintain my working branches locally and will not revert my remote main branch to be back in-line with publiclab until I am certain that your work will not be lost. I will also make a copy of my main branch with the commits you have pushed to it before I revert it back.

Please let me know when your work is secure.

@Forchapeatl
Copy link
Collaborator Author

@stephaniequintana I am sorry for the inconvenience. I have the work saved. Please overwrite it.

@Forchapeatl
Copy link
Collaborator Author

Hello @stephaniequintana this week has been confusing to me. How are you doing ? Hope all is well ! Please for a run down of the Yesterday's meeting. And A Process flow of how to perform "preloaded images" on the UI. (Or do you mean drag and drop ?).
Excellent Job by the way!

@Forchapeatl
Copy link
Collaborator Author

Hello @stephaniequintana hope you are doing well. I am unable to open an issue. I will just keep it here . So we don't forget. Full-screen assumes a smaller div size after exit

e9088fb2-343d-44d4-bb8a-3344eab27888.mp4

This line

<canvas class="fullscreen" id="image">

is quite different from the original implementation

<canvas class="fullscreen" id="image" width="800" height="600"></canvas>

I understand you left it out so the website could be responsive. Can we set a default height and width styling to the canvas element ?

@stephaniequintana
Copy link
Owner

Hi, @Forchapeatl - I have been concerned with the canvas dimensions since I began coding the new UI. I know that there are several functions which rely on it and that in general it should be set within the html element, just as you referred to above.

Having it set to such a large dimensions, 800 X 600, causes a very undesired results with the new UI. If we keep it with these dimensions I may need to rethink the layout. This is fine if we need to keep it at 800 X 600 for functionality, but I'm hoping we can work around this. 🤞

  • Q: Do you foresee any issues with us using smaller dimensions? I haven't tested any yet, and I haven't looked at all of the javascript, but it seems that that sizing/re-sizing the image is dependent on the dimensions listed in the html where we use the imgCanvas.width and imgCanvas.height, like in this function:
function updateImage(img) {
        var ctx, height, imgCanvas, width;
        imgCanvas = document.getElementById("image");
        ctx = imgCanvas.getContext("2d");
        width = img.videoWidth || img.width;
        height = img.videoHeight || img.height;
        ctx.drawImage(img, 0, 0, width, height, 0, 0, imgCanvas.width, imgCanvas.height);
        image = ctx.getImageData(0, 0, imgCanvas.width, imgCanvas.height);
        return set_mode(mode);
      }

Please let me know if you think using smaller dimensions will cause other issues. In the meantime I will play with a few to get a feel for what might work best to help keep the layout at 100vh.

@jywarren
Copy link
Collaborator

jywarren commented Aug 2, 2022

I am unable to open an issue.

Hi all, just wanted to apologize for the permissions issues, I had wanted to try to assign group permissions but it just isnt' working so I just sent you both a direct invite to full write permissions for Infragram. Although i wonder -- Forcha maybe you are talking about making issues within Stephanie's repository? Could we just make issues in publiclab/infragram even if the discussion is of someone's fork? That way they are all in once place and we don't miss any. But I'm flexible and open to suggestions, sorry if I misunderstood something!

The fixed vs. fluid canvas size thing is tough. However, there are ways to have a canvas have a fixed # of pixels width but be displayed in a "stretched" manner and occupy a different number of pixels on the actual device screen. Think of how a large image can be stretched/squashed down to fit on a smaller width window.

I looked around for some guidance on this and found this which may help:

https://dev.to/robotspacefish/resizing-html5-canvas-and-scaling-sprites-5cpe

So, keep in mind that there is a distinction and there are multiple ways to set and display dimensions of a canvas. Please let me know if this is confusing or if I can help more!

@stephaniequintana
Copy link
Owner

@Forchapeatl - I'm adding this discussion link here as a reminder to us both...

  • Moving forward with currently open PRs and all future work, please add/include the version flag to any v2 function that will break v1.
  • We'll eventually move all functions from infragram2.js to infragram.js so that each version will run off of only infragram.js

I suggest that we wait to address combining all the remaining functions until after your latest work is merged.

I hope you're doing well!

@jywarren
Copy link
Collaborator

jywarren commented Oct 11, 2022 via email

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 this pull request may close these issues.

3 participants