[Review request] anyone have some time to review a PR i made

@ReactMeteor anyone have some time to review a PR i made.

I’ve removed a section on the homepage and added two new ones.

I’m a beginner with ReactJS and can’t see why its not working.

Actually, failing to update this myself led me to make the very well-received post on Reddit… and i’m feeling a little embarrassed that our homepage isn’t really showing casing our projects very well or accurately to everyone who’s joined

Sad In A Box GIF

Hey! I just found this project the other day and haven’t really familiarized myself with the code base, but scrolled briefly through your PR. My React experience is fairly limited so I can’t point out any glaring issues, but I know 2 heads are (sometimes) better than one. If you wanted to hop on a call to talk through and try to solve the issue together, I’m happy to do so.

1 Like

Sure, that’s a great idea! I’ve been travelling a lot today for an AI conference so my apologies if i look a bit sleepy.

I was thinking i should suggest some calls to help people get set up and rolling so your suggestion is an excellent start :slightly_smiling_face:

Here’s a meet link: meet.google.com/jrh-qwrg-ond

@justenanderson thanks for having a look with me tonight. I had definitely missed that second ‘emerald’ colour code so thats one issue fixed. Still something the app doesn’t like so i’ll revert changes for now and make a PR which yourself and others can review before pushing again.

Here’s the error code in console:

lockdown-run.js:17 Lockdown failed: TypeError: At intrinsics.Object.groupBy expected boolean not function
at isAllowedPropertyValue (lockdown-install.js:1:53384)
at isAllowedProperty (lockdown-install.js:1:53807)
at visitProperties (lockdown-install.js:1:55095)
at isAllowedPropertyValue (lockdown-install.js:1:53041)
at isAllowedProperty (lockdown-install.js:1:53807)
at visitProperties (lockdown-install.js:1:55095)
at lockdown-install.js:1:55523
at repairIntrinsics (lockdown-install.js:1:144597)
at lockdown-install.js:1:145462
at lockdown-run.js:4:3
(anonymous) @ lockdown-run.js:17
lockdown-more.js:99 Protecting intrinsics failed: ReferenceError: harden is not defined
at lockdown-more.js:69:13
at Set.forEach ()
at protectIntrinsics (lockdown-more.js:44:22)
at lockdown-more.js:97:5
(anonymous) @ lockdown-more.js:99
deprecated.js:52 [PLUGIN discourse-custom-wizard] Deprecation notice: Importing getOwner from discourse-common/lib/get-owner is deprecated. Use import { getOwner } from '@ember/application', or if you still need the fallback shim, use import { getOwnerWithFallback } from 'discourse-common/lib/get-owner';. [deprecated since Discourse 3.2] [deprecation id: discourse.get-owner-with-fallback]
o @ deprecated.js:52
plugin-api.js:143 [PLUGIN discourse-custom-wizard] To prevent errors in tests, add a pluginId key to your modifyClass call. This will ensure the modification is only applied once.
Se @ plugin-api.js:143
lockdown-run.js:17 Lockdown failed: TypeError: At intrinsics.Object.groupBy expected boolean not function
at isAllowedPropertyValue (lockdown-install.js:1:53384)
at isAllowedProperty (lockdown-install.js:1:53807)
at visitProperties (lockdown-install.js:1:55095)
at isAllowedPropertyValue (lockdown-install.js:1:53041)
at isAllowedProperty (lockdown-install.js:1:53807)
at visitProperties (lockdown-install.js:1:55095)
at lockdown-install.js:1:55523
at repairIntrinsics (lockdown-install.js:1:144597)
at lockdown-install.js:1:145462
at lockdown-run.js:4:3
(anonymous) @ lockdown-run.js:17
lockdown-more.js:99 Protecting intrinsics failed: ReferenceError: harden is not defined
at lockdown-more.js:69:13
at Set.forEach ()
at protectIntrinsics (lockdown-more.js:44:22)
at lockdown-more.js:97:5
(anonymous) @ lockdown-more.js:99
142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:287 gatherings
142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66 TypeError: Cannot destructure property ‘title’ of ‘k’ as it is undefined.
at E.render (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:278:30492)
at ic (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:70607)
at oc (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:70402)
at _s (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:105738)
at Us (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:97544)
at Bs (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:97469)
at Ls (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:94394)
at js (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:91159)
at vl (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:112632)
at 142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:113748
xc @ 142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66
142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66 Uncaught TypeError: Cannot destructure property ‘title’ of ‘k’ as it is undefined.
at E.render (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:278:30492)
at ic (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:70607)
at oc (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:70402)
at _s (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:105738)
at Us (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:97544)
at Bs (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:97469)
at Ls (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:94394)
at js (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:91159)
at vl (142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:112632)
at 142763e4d5c980a10a6915db1b14ac1355dd04c8.js?meteor_js_resource=true:66:113748
js?key=AIzaSyCuiq_ahPzWE02eCsyaquoVABPUGszpLGM&v=3.exp&libraries=places:233 Google Maps JavaScript API has been loaded directly without a callback. This is not supported and can lead to race conditions and suboptimal performance. For supported loading patterns please see Genel bakış  |  Maps JavaScript API  |  Google for Developers

Hey just waiting on settings.json access and then I will take a look. I’m intermediate xp and can already see some noticeable errors so maybe we can get this sorted out.

Thanks!

1 Like

I approved your request. Great that you’ve found some issues already!

Thanks @AndyatFocallocal! Do you happen to have a link to the PR, or can we possibly set up a quick call some time this week to get me up to speed with the repo in general? I have the environment up and running, just not familiar with Discourse and if I’ll need to set that up as well for the React tasks. My full time doesn’t currently involve any repos per say so my navigation of GIT is a tad rusty, and a point in the right direction would help a ton. Thanks again for the opportunity to be a part of this cause.

1 Like

Just directly to the Commit. I’ll push it all to the Andy2 branch to make it easier.

Yes I was thinking some video calls would be good but I’m away at a conference atm. How about we find a time this Sunday?

It’s great to have you here ,:slightly_smiling_face:

1 Like

I got it, checked out your commit. Thanks! Sunday works for me, lets keep in touch. Have a great time at the conference, those are fun.

1 Like

@nyne87 and @justenanderson i’m having another go at this today, are either of you available to help?

The only thing i can currently think of is that i should have created a completely new folder for both new sections, rather than cannibalising the already existing ‘Movements Section’ folder

Here’s a more detailed write up of the problem:

Here’s the homepage

I’m a beginner with ReactJS and can’t see why its not working.

I wanted to remove Section 2 on the homepage and added two new sections which are clones of the Projects Section, so the page has a banner image and then three sections with a title then boxes underneath.

The project uses i18n to define which text to show to the user, so any visible text or images are added in the i18n folder, so i’m working on two files which operate together.

I changed the ‘Movement Section’ into a clone of the Projects Section, and created a new section called Events.

Perhaps i should have left the ‘Movements Section’ alone and just commented it out, then created a new folder for both new sections rather than cannibalising the existing one?

I also asked ChatGPT to suggest some colours to use and it turns out its crap and Emerald is not a recognised colour in browsers. I’ve removed that but the commit still won’t load.

Hey @AndyatFocalloca, I didn’t see this until now. I need to enable notifications for this app. I’m going to take a look at this tomorrow. Thanks for the extra details, I’ll ping you with any questions.

1 Like

Found the issue. I checked out your commit, changed the title within home.json from “movements_section” to “movement_section” as it was being referenced within MovementSection/index.js and it loaded right up.

image

I didn’t push anything up and submit a PR as I’m a bit rusty with git (my full time doesn’t use it, and I’ve lost some of my chops) and your changes are in a prior commit, etc. Let me know what you’d like me to do or if you want to just change that line yourself :+1:

Have a great night/day, wherever you are haha.

1 Like

Let me give it a try, it matches with the error message research I’ve been doing.

Thanks so much, I was pretty stumped on it

Finally! Thank you so much @nyne87, i knew it was going end up being something small that i’d overlooked. I’ll set to work on updating the photos in those sections when i get home tonight :slight_smile:

Very welcome! It’s the little things that can sometimes cause the biggest headaches haha.

I do have a question though about the tests, whenever I run them locally I get the below message:

SecurityError: localStorage is not available for opaque origins

Have you encountered this before? Seems like I’m missing a config file or something.

1 Like

The tests have been built and rebuilt a few times by different people. Right now they aren’t operating correctly, so we’re not paying a lot of attention to them until someone decides to fix them again.

1 Like

Ok, was hesitant to try and fix as it appears to be a dependency issue with Jest/enzyme and or something in the jest config. I will try and fix it locally and see what I come up with.

1 Like