publiclab / infragram

A minimal core of the Infragram.org project in JavaScript
https://infragram.org/sandbox/
GNU General Public License v2.0
44 stars 166 forks source link

V2 functions connect #431

Closed stephaniequintana closed 2 years ago

stephaniequintana commented 2 years ago

This branch is a combination of the added functionality that @Forchapeatl has completed and an updated UI for the sandbox page of Infragram.org. It includes the changes addressed in #0427 and #0421 and has multiple additional changes. A live representation can be viewed at https://stephaniequintana.github.io/infragram/index2.html#

We agree that this branch provides a solid foundation to build upon as our projects progress. It contains changes to the

Moving forward, we would like to merge this branch and then make further changes to the files directly through individual PRs.

We would very much appreciate feedback, questions, ideas...as this PR holds multiple commits and encompasses many new features,

We look forward to you thoughts!

Screen Shot 2022-07-06 at 8 31 34 PM

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

gitpod-io[bot] commented 2 years ago

jywarren commented 2 years ago

This looks great and I love the progress so far! A lot of the input i have is very focused on one thing or another, so maybe best for after we merge this? For example:

But overall this is looking great, i'm happy to merge if you're both ready?

image
stephaniequintana commented 2 years ago

@jywarren, thank you for the feedback &&& I agree.

As is, it is not inherently clear how to best utilize the site/tools. I cannot locate the specific comment, but in an earlier version @TildaDares made some great suggestions on how to better convey the initial steps. Also, I think adding in the welcome/navigation tour and labeling the "source" icons will help immensely. I am still working out the best flow for myself and am hoping that the UI will improve as I progress.

To the font-size, there are/were issues with the card-container itself overfilling the image-container on resize (both resizing the font and the window), but I have had an epiphany as to why this is occurring. Taking in @TildaDares's suggestions and also possibly moving the current welcome text to the welcome/navigation tour, I'll be able to address the font size with no issues.

I believe we are ready to merge this. I know that @Forchapeatl is already working on correcting a toggle function and it would be great if she will be able to open a PR for that directly against this.

Moreover, I understand that this specific PR still has issues of its own. We will very much depend on feedback to help us identify some of those as well as the overall best approach for our users.

Thanks again, @jywarren, I'm excited to see this develop!

jywarren commented 2 years ago

OK, exciting!!!! Great work everyone!!

stephaniequintana commented 2 years ago

Oof, @jywarren, I just realized I was a bit careless in what I had pushed to this v2_functions_connect branch...there is a file, indexF.html which does not belong. My apologies.

If I can correct this, please let me know how - I believe it can only be deleted by someone with write access to the repo. Again, I apologize for overlooking this.

jywarren commented 2 years ago

Hi! No worries, you can just submit a new PR removing it?

On Thu, Jul 7, 2022, 3:37 PM Stephanie Quintana @.***> wrote:

Off, @jywarren https://github.com/jywarren, I just realized I was a bit careless in what I had pushed to this v2_functions_connect branch...there is a file, indexF.html which does not belong. My apologies.

If I can correct this, please let me know how - I believe it can only be deleted by someone with write access to the repo. Again, I apologize for overlooking this.

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

stephaniequintana commented 2 years ago

Yes, that. I don't understand how to do that. If I submit a PR, a request to merge a branch (with that file removed), then when merged with the main branch that file will still exist bc it's already in the main branch (?)

Perhaps my brain is tired and not processing correctly. It is my understanding that the file can only be removed from the CLI with someone who has access to the repo.

You are suggesting that if I git rm indexF.html and open a PR, merging that will in effect accomplish removing it from the main branch?

Forchapeatl commented 2 years ago

Hello @stephaniequintana I think this is the easiest way selection

stephaniequintana commented 2 years ago

@Forchapeatl, thank you. Unfortunately, that avenue will only delete the file in my own personal fork. If I do this while on a new branch and open a PR for that branch then the file will be removed from main when merged?

Ugg. I, guess this is one way to learn (not my preferred way, btw) - thank you all for your patience.

jywarren commented 2 years ago

If I do this while on a new branch and open a PR for that branch then the file will be removed from main when merged?

Yes, that's right! Sorry i didn't see your message, was offline writing for a bit.