Skip to content

JS compatibility updates and minor install.md updates #25

Closed
wants to merge 5 commits into from

2 participants

@samurai
samurai commented Apr 7, 2013

This seems to want to include the install guide itself (perhaps the #22 PR wasn't accepted yet?), nonetheless there is also a small modification to that included in this.

This is mostly JS updates to the way grefs are drawn and updates are pushed so that it works for firefox. I've done basic testing on these changes and believe them to be cross-compatible with most things, tho IE seems to not like it

@richo
Owner
richo commented on df93add Apr 3, 2013

The comment would be better off in the commit message. Commit messages are forever, source comments are transient.

It is :)

I've done some testing (basic), but I believe this fix should be cross-compatible with most browsers (whatever version of IE I currently have running tho doesn't like it tho ...)

@richo
Owner
richo commented Apr 7, 2013

Yup, you generally want to start new features off master git checkout -b features/foo_thing master

I'll have a look over this and probably rebase it into master.

@richo

Did you work out what to do about whitespace with this? When I try this it maims my markdown.

@richo
Owner
richo commented Apr 7, 2013

I rebased 1380c00 in e17076b so that can come out of this PR.

The rest I'll need to check over, but I'm pretty sure text()

@richo richo commented on the diff Apr 7, 2013
airship/static/airship.js
@@ -112,7 +112,7 @@ var GrefMenuItem = Backbone.View.extend({
select: function() {
var active_el = this.el;
- $(this.el.parentElement.children).removeClass("active");
+ $(this.el.children).removeClass("active");
@richo
Owner
richo added a note Apr 7, 2013

I just tested in FF and chrome, this Just Works for me.

What are you seeing?

@samurai
samurai added a note Apr 10, 2013

I get no content, it errors out on that element not existing. When enumerating the items in this.el, parentElement doesn't show up in the FF builds I tested under. this.el.children seemed to be supported across all browsers I tested, and presumably contains identical datas

@richo
Owner
richo added a note Apr 10, 2013

something weird is happening :/

Maybe

$(this.el.children || this.el.parentElement.children)

until we can work out why it's doing that? What ff version are you testing on? this.el.children was broken everywhere I tried it.

@samurai
samurai added a note Apr 10, 2013

As it turns out, that one I only tested under a really old version of FF =\ disregard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samurai
samurai commented Apr 10, 2013

@richo , forgot about the whitespace issue with that. If you haven't already looked into it, I'll knock that out

@richo
Owner
richo commented Apr 10, 2013

@samurai I ported all the markdown'y bits to just use textareas: https://github.com/richo/groundstation/pull/23/files

@richo
Owner
richo commented Apr 10, 2013

Closing, I'm pretty sure this is all on master now.

@richo richo closed this Apr 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.