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

Make it an object (fixes #5) #7

Merged
merged 18 commits into from
Mar 8, 2019
Merged

Make it an object (fixes #5) #7

merged 18 commits into from
Mar 8, 2019

Conversation

LoganDark
Copy link
Collaborator

@LoganDark LoganDark commented Mar 8, 2019

Incorporates the parent method from #6 (sorry, referenced issue 4 accidentally before)


  • change this to an object (you can now store selections for later and still operate on them)
  • switch to tabs (reduces file size, allows people to choose their indent size without affecting others, only uses one character per level, spaces can be used for alignment)
  • add sel method for getting the now-scoped selection as a regular array of nodes
  • Breaking change: offset() now returns the offset of the first element rather than setting a global variable to the offset of the last
  • getAttr() now works, it returns the attribute gotten from the first element rather than being a no-op
  • thanks to the fact that the selection is now an Array, the minified file can now use forEach because it's shorter than the for loops, but the downside is that you can't use automated minification anymore

This does not use proper classes with prototypes and things, because I was lazy. This is better than what the library was previously in any case. Let me know if you'd like me to revert any of these changes. This is completely untested though, so merge at your own risk

Note, this does not update the lib directory

@LoganDark
Copy link
Collaborator Author

Fixes #5 (GitHub does not register the reference in the title for some reason)

@LoganDark LoganDark mentioned this pull request Mar 8, 2019
@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Does it support chaining?

Example: $("div").css("background-color:green;").html("Hello World")

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

The methods that don't return a value support chaining, yes. Methods with return this in the code allow you to chain afterwards since they return the same object the method was called on.

In any object, obj.x() calls x() with obj as the this.

@LoganDark
Copy link
Collaborator Author

I should remind you that this code is entirely untested.

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Try to test this with your code: $("div").css("background-color:green;").html("Hello World");

@LoganDark
Copy link
Collaborator Author

I was half-expecting you to test it for me since you're the maintainer and you know what's best, but sure, I'll try that.

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

I just did :)

@LoganDark
Copy link
Collaborator Author

image
I think it works.

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Yes the css will work, what about html? Every div should display: "Hello World".

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

Good point. I'll look into it

@LoganDark
Copy link
Collaborator Author

So it turns out arrow functions preserve the this of the function they were defined in. That's fine, I'll use regular anonymous functions. Commits incoming.

@LoganDark
Copy link
Collaborator Author

image
On GitHub the new version produces the expected result.

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

Okay, I made a few updates. I made chaining work and added more argument types. You can now do stuff like

$(<HTMLElement of your choice>)
$(<Array of selectors>)
$(<Array of HTMLElements>)
$(<Array of any other argument the function accepts, including more arrays>)

Yay

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Ok.. I hope the result is different from my previous project https://github.com/vladocar/nanoJS/blob/master/src/nanoJS.js :)

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

Yep, it is, don't worry :) I'd love to become a maintainer here if that's possible!

I tested both the minified and non-minified versions of my latest commit and they both work.

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Ok, much better 👍

Now I need to rewrite the docs and I will approve it..

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

You won't have to rewrite them fully, I hope.

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

I could rewrite them for you if you like. Where are they stored? I could add them to the PR

Edit: Nevermind, they're on a different branch. It's identical here though. I could add the latest commit from gh-pages to master, do this thing and then you could merge it back into gh-pages?

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Unfortunately I do. All in the interest of not polluting the global object.

@LoganDark
Copy link
Collaborator Author

Do you have any IM like Discord? This PR is getting loooong

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

Yeah you are right. Just Skype.. I hope to finish with the docs today.

@LoganDark
Copy link
Collaborator Author

Make a Gitter then?

@vladocar
Copy link
Owner

vladocar commented Mar 8, 2019

gitter.im/femtoJS

@LoganDark
Copy link
Collaborator Author

LoganDark commented Mar 8, 2019

Did a bunch of stuff, maintainer approved, waiting on merge now

@vladocar vladocar merged commit b9dc72b into vladocar:master Mar 8, 2019
@LoganDark LoganDark deleted the patch-2 branch March 8, 2019 17:15
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

Successfully merging this pull request may close these issues.

2 participants