Rename main_image.js to mainImage.js

[larabee7777]

Fixes #9148

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

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updation
• ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

[gitpod-io]

[larabee7777]

@noi5e please review when you get a chance thanks :)

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@f87860c). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #9201   +/-   ##
=======================================
  Coverage        ?   81.97%           
=======================================
  Files           ?      100           
  Lines           ?     5953           
  Branches        ?        0           
=======================================
  Hits            ?     4880           
  Misses          ?     1073           
  Partials        ?        0           

[Manasa2850]

Hi @larabee7777 could you please add the issue number at Fixes #0000 (<=== Add issue number here). It'll help us to automatically close the issue once this PR is merged.
Thank you!

[noi5e]

@Manasa2850 Ah yeah, good catch! @larabee7777 I didn't know you had this PR open because it wasn't linked to the issue. Putting the issue number in the PR links it back there.

It looks like the tests are failing, so we won't be able to merge this yet:

If I look under Files Changed...

I see that you renamed the file, but didn't do the other change that was in the issue. Please make another commit to include that change! Thanks.

[larabee7777]

Hi, I think I separated the two issues into 2 pull requests because it looked like two issuse - one code change and one file name change. So I thought I did both issues, but let me look again and see what I missed.

/app/assets/javascripts/application.js - went to this file and edited the code

• //= require mainImage.js

• //= require main_image.js
  /app/assets/javascripts: - i went to this list of file names and edited the file name to match my previous change so that the names match

• mainImage.js

• main_image.js

So I think I at least understood what was expected of me but I made a mistake somewhere and I must check again

[larabee7777]

So I checked both pull requests and linked them as requested, in that main description area.

So these two PR's fix #9148

publiclab/plots2 Rename main_image.js to mainImage.js
#9201 opened 15 hours ago by larabee7777 • Review required 0 of 5
1
7
publiclab/plots2 Update application.js
#9200 opened 15 hours ago by larabee7777 • Review required 0 of 5

Should it be one PR per issue? If so how would I have done that because I thought I had to branch off each filename and each change so that if one fix is right and the rest wrong we can approve the one right action and work on the incorrect ones.

Please let me know what I did wrong and how I can improve, thanks so much for your help!

[larabee7777]

It looks like the tests are failing, so we won't be able to merge this yet:

I dont know what those tests mean please help me with that. The code is commented out so I thought I had to leave it as is and the errors would be fixed as other issues

[noi5e]

@larabee7777 Thanks for hanging in there and asking for clarification!

It has to be 1 PR containing both changes simultaneously. If the changes are spread out over 2 PRs, it's like there are 2 different versions of the app. Each pull request has its own version of the app, and they have no relation with each other.

If you rename main_image.js to mainImage.js, then application.js needs to be updated as well. If you don't update application.js, then what happens is it will try to look up main_image.js like always... And won't find anything (because the file has been renamed).

If you look through the failed tests (click on Details in the screenshot I took above), you can get a sense for this by looking at the readout:

Aha! It says ActionView::Template::Error: couldn't find file 'main_image.js' with type 'application/javascript'. That shows me that the theory above is probably correct. I know that it can be kind of confusing sifting through failed tests, but you really can learn a lot that way! 😊

I was also confused about lines looking like they were commented out in application.js. For confusing things like that, I always find it's helpful to Google documentation to learn about it. Searching for "application.js rails" comes up with this for example. I still don't really understand why Rails does it this way, but it does tell me that it's working, and is not just a comment.

Hope that helps! So definitely make 1 PR with both changes and we'll take it from there. Actually you don't need to open up a new PR, you can just push the commit to the PR you already have open. In other words, make the change to application.js in the patch-2 branch here. Let me know if anything else comes up!

[codeclimate]

Code Climate has analyzed commit 0866bfe and detected 0 issues on this pull request.

View more on Code Climate.

[larabee7777]

It looks like both changes are on the same PR now as I see two file changes

One question:
$(document).ready(function(){
$('#image_revision').change(function(){
$('#main_image').val($('#image_revision option:selected').attr("id"));
$('#leadImage').attr("src",$('#image_revision option:selected').val());
});
});

this is the code in the file mainImage,js - the filename was changed so must I change that main_image code in line 3 with the correct camel case mainImage too? I didn't see explicit instructions to change that so I left it.

[noi5e]

@larabee7777 Ah no, that's different. That's an example of a jQuery selector. jQuery is referring to an HTML element that has an ID #main_image, different from this file that we're renaming.

Come to think of it, this is probably a good example of why programmers have naming conventions. Usually HTML elements follow the convention of #main-image instead of #main_image, which is the Ruby on Rails convention. In this case, it's confusing! 😕

Otherwise, this looks good! Your tests are all passing! That means this is ready to merge!

[jywarren]

Great work folks! Thanks so much @larabee7777 and thanks @noi5e for the support!!!

[welcome]

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to PublicLab.org in the next few days, but first it will be published to https://stable.publiclab.org/ (it will take some minutes for this to load, and until then you may see logs from the build process). Please test out your work on this testing server and report back with a comment that all has gone well!
Do join our weekly check-in to share your this week goal and the awesome work you did 😃. Please find the link to our latest check-in here 📝
Now that you've completed this, you can help someone else take their first step!
Reach out to someone else working on theirs on Public Lab's code welcome page (where you'll now be featured as a recent contributor!). Thanks!

Help others take their first step

Now that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌

https://code.publiclab.org

Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕

People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉

Read about how to help support another newcomer here, or find other ways to offer mutual support here.