Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should support .css("+xyz") and .css(">abc") #621

Closed
dlee opened this issue Feb 19, 2012 · 14 comments
Closed

Should support .css("+xyz") and .css(">abc") #621

dlee opened this issue Feb 19, 2012 · 14 comments

Comments

@dlee
Copy link
Contributor

dlee commented Feb 19, 2012

In jQuery, it's convenient to try looking for a node that follows another by doing:

<div>
  <dt>blah
  <dd>blah
  <dt>blah
  <dd>blah
</div>
$("dt").find("+dd")

The above example is contrived, but it's just an illustration

It would be nice if nokogiri also supported +sibling notation on the current node.

**Update:* Nokogiri should also support .css(">abc") as well, and it'd probably be the same fix, so I'm adding it to this bug.*

@dlee
Copy link
Contributor Author

dlee commented Feb 19, 2012

From a naive approach, it seems like interpreting +xyz and +abc as if there's a * in front seems to work for NodeSets. My current workaround is to append * in front, like this: nodeset.css(*+xyz).

@dlee
Copy link
Contributor Author

dlee commented Feb 19, 2012

Actually, the above approach of prepending a * was indeed too naive. It returns matches not only relative to the nodes in the nodeset, but relative to all descendant nodes. For now, I can't think of a workaround.

@flavorjones
Copy link
Member

Howdy,

We'd like to support as much JQuery syntax extensions as we reasonably can. Can you attach a test case that illustrates how this feature should work? That would save Team Nokogiri some time.

Thanks!

@dlee
Copy link
Contributor Author

dlee commented Feb 21, 2012

I submitted pull request #623.

How do you submit a pull request for an existing GitHub issue?

@dlee
Copy link
Contributor Author

dlee commented Feb 22, 2012

@flavorjones Sorry, should have been clearer. The test cases are in pull request #623, as well as the fix.

@flavorjones
Copy link
Member

@dlee - Awesome, thanks. Probably won't be merged in for 1.5.1, but the next version.

@dlee
Copy link
Contributor Author

dlee commented Feb 23, 2012

@flavorjones Cool. What's the schedule for 1.5.1 and 1.5.2?

@flavorjones
Copy link
Member

1.5.1 is imminent. After that, we're not sure. Certainly not seven months again. ;)

@dlee
Copy link
Contributor Author

dlee commented Apr 19, 2012

I see that 1.5.3 is at rc3. Will the pull request make it for that release?

@flavorjones
Copy link
Member

Apologies ... I will spend some time today on your pull request. Thanks for keeping on top of us!

@dlee
Copy link
Contributor Author

dlee commented Apr 26, 2012

@flavorjones any updates?

@flavorjones
Copy link
Member

#623 has been merged! I'll put together an RC in the next day or so.

Thanks so much!

@dlee
Copy link
Contributor Author

dlee commented May 26, 2012

Sweet! When does this issue get closed?

@tenderlove
Copy link
Member

#623 was merged, so I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants