newhavenio / newhavenio.github.io

active version of the website for newhaven.io built on the Jekyll framework
http://newhavenio.github.io/
MIT License
13 stars 12 forks source link

Refactor Meetup.com codebase #10

Closed sukima closed 7 years ago

sukima commented 7 years ago

Oh boy this was a doozy. Sorry it took so long but work and family kept getting in the way.

Coming from Ember.JS (and this is probubly true for Angular devs too) the push to components for rendering part of a page is quite prominent. Much of Ember is trying to close the gap between the web components standard and the do-it-yourself frameworks.

Using those ideals it seemed appropriate to look into the web component standard. The concepts were perfect for the problem domain on the website. So I dove in. I wanted to have a safety net so I also worked on developing a test suite.

After it was all done I also looked into a way to search for developers. Sadly though the Meetup API does not support text searching members.

Another edge case I found was that the paginator (even the original one) doesn't scale well when you get enough members to cause the list to wrap to the next line. Perhaps a new paginator design would help or maybe some fancy flexbox work. If anyone has thoughts on this perhaps we can discuss it here.

maxx1128 commented 7 years ago

Looks and works great! Understandable that you had other priorities before getting to it. The only issue I see is the mobile menu isn't opening for some reason. There was a jQuery-related error in the console that's likely connected, is below.

TypeError: a.getAttribute is not a function

Once that's set and the mobile menu is fixed then it'll be good to merge. As for the paginator and general layout, I can add some flexbox later on so it handles better on mobile and other screen sizes.

kljensen commented 7 years ago

👏👏

sukima commented 7 years ago

@maxx1128 I am unable to reproduce this error on commit 9f7b25485f9cdc4069abc4df1bef3d4bb6f48a11. Can you help diagnose this?

nhio-no-error

maxx1128 commented 7 years ago

Looks like it's only happening on Firefox for me. Not sure if this is only because it's being run locally, took a shot of the error messages below.

screen shot 2016-08-09 at 8 27 08 am
sukima commented 7 years ago

@maxx1128 Again I can't seem to make this happen. Really sorry.

newhavenio-firefox

(FireFox is slow when dev tools are open. Oh FireFox.)

treznick commented 7 years ago

@max1128 are you running any browser plugins that maybe would be interfering with things? Does this still work in a clean Firefox?

On Tue, Aug 9, 2016 at 1:27 PM, Devin Weaver notifications@github.com wrote:

@maxx1128 https://github.com/maxx1128 Again I can't seem to make this happen. Really sorry.

[image: newhavenio-firefox] https://cloud.githubusercontent.com/assets/70075/17526334/d1a38b70-5e34-11e6-806e-7b7b35a7a28a.gif

(FireFox is slow when dev tools are open. Oh FireFox.)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/newhavenio/newhavenio.github.io/pull/10#issuecomment-238626873, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlwA06_x2eFO7imUqp6vPgDja-ekx7mks5qeLhkgaJpZM4Jej4k .

maxx1128 commented 7 years ago

Cleaned out Firefox but is still occuring. @sukima was it still working on your firefox when you tried the mobile menu?

If it's only my Firefox having an issue here though, I say we merge anyway. I'd only say we need to resolve it if it's on all Firefox browsers. But if the menu is working on Firefox for everyone else then I'm good to merge.

treznick commented 7 years ago

@sukima @max1128 I'm able to replicate it locally.

@sukima, let's look at this tomorrow morning.

On Tue, Aug 9, 2016 at 3:31 PM, Max Antonucci notifications@github.com wrote:

Cleaned out Firefox but is still occuring. @sukima https://github.com/sukima was it still working on your firefox when you tried the mobile menu?

If it's only my Firefox having an issue here though, I say we merge anyway. I'd only say we need to resolve it if it's on all Firefox browsers. But if the menu is working on Firefox for everyone else then I'm good to merge.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/newhavenio/newhavenio.github.io/pull/10#issuecomment-238664723, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlwA-CPeN2hPHl0Nmg2ZbJGqXxSFFO4ks5qeNWOgaJpZM4Jej4k .

sukima commented 7 years ago

Ok I found it. Took a while sorry 'bout that. https://github.com/webcomponents/webcomponentsjs/issues/49

Will look into a fix…

sukima commented 7 years ago

@maxx1128 Try it now.

maxx1128 commented 7 years ago

It's now working for me! Will merge it later today.

sukima commented 7 years ago
Boom that's how its done Chuck Noris style
treznick commented 7 years ago

🎉