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

Hinclude can evaluate JS based on an attribute #28

Closed
wants to merge 2 commits into from

Conversation

fabiofabbrucci
Copy link
Contributor

Hi @mnot,
I create this PR to let the element hx decide if evaluate or note the JS.

I also created tests for this.
let me know if it interests or not.

PS.
This commit is the base for creating a test for "refresh" function.

@damienalexandre
Copy link

👍

@fabiofabbrucci
Copy link
Contributor Author

So @mnot, what about this PR?
Is there something that you don't like it?

@mnot
Copy link
Owner

mnot commented Mar 12, 2015

Actually, it looks really good; sorry for the extended delay.

Two things that should at least be discussed first:

  1. Should we check the type attribute on the script element? Non-JS scripts are becoming more common.

  2. What about script src?

@fabiofabbrucci
Copy link
Contributor Author

nice.

  1. which kind of scripts are you thinking about? Anyway yes, we could filter only javascripts type by attribute "type"
  2. Are you talking @mnot about adding a data-attribute that specify which JS file run after render?

@mnot
Copy link
Owner

mnot commented Mar 15, 2015

  1. e. g. coffeescript- http://coffeescript.org/#scripts or vbscript etc.

  2. no I mean <script src="..."></script>

@mnot
Copy link
Owner

mnot commented Mar 15, 2015

Arg html. Script src="..."

@mnot
Copy link
Owner

mnot commented Nov 12, 2015

Please re-base and I'll incorporate. I'll address the first issue above after that; the second I think we can just document.

@mnot
Copy link
Owner

mnot commented Nov 12, 2015

Should fix #23

@nurikabe
Copy link

+1

1 similar comment
@pedroresende
Copy link

+1

@pedroresende
Copy link

@mnot is this still maintained ?

@mnot
Copy link
Owner

mnot commented May 6, 2016

@pedroresende yes; as per above, it needs to be rebased.

@pedroresende
Copy link

@mnot more than one year ago ??

@peter-gribanov
Copy link
Contributor

it would be better to use events #50

@Wirone
Copy link

Wirone commented Mar 22, 2017

@peter-gribanov events are one way for achieving something dynamically, but this is useful too. Sadly, repo owner is probably most user-unfriendly (not to say more) guy I met on GitHub so this little change couldn't be incorporated for almost 4 years. *

We use hinclude with Symfony fragment renderer so sometimes I need to deal with it, but we have custom version (1.1.0 + this PR + own changes like skipping initial loading if fragment is rendered with default content) because we don't want to struggle with PRs for months.

* Even though I am really disappointed that this feature is still not merged and I don't understand why @mnot waited for rebasing so long when he could do this very quickly and add useful feature to his library, I shouldn't use those exact words.

@mnot
Copy link
Owner

mnot commented Mar 22, 2017

@Wirone it needs to be rebased. Instead of calling me names, why don't you create a PR?

@Wirone
Copy link

Wirone commented Mar 22, 2017

@mnot If you were user-friendly you could rebase it on your own because this is helpful feature but PR author is not active and didn't do it. But instead you're waiting for months/years and this is not the first time as far as I watched this repo. I would understand not working on feature requests if you don't have time, but here you've got ready feature and yet you're waiting for someone.

PS. It took me about 2 minutes to include these changes in our customized 1.1.0 version.

@mnot
Copy link
Owner

mnot commented Mar 22, 2017

So, let me get this straight -- you're using Open Source software, and you've created your own fork, but you don't upstream your changes, and you blame the repo owner for not doing that work for you?

I'm a bit busy. If you don't want to use this software, that's fine by me. If you want to create a fork, that's great -- we already have one that I actively support/link to. Just don't waste my time by complaining.

You might want to read something a friend of mine wrote about this:
https://hueniverse.com/2016/01/26/how-to-use-open-source-and-shut-the-fuck-up-at-the-same-time/

@Wirone
Copy link

Wirone commented Mar 22, 2017

We don't have a fork but customized script in own repo on company's GitLab.

The fact this is OpenSource does not mean that I can't complain about situation when you can't rebase it for PR's reporter when he didn't do it for long time. Today I needed this feature and was totally confused about your attitude. I could rebase it and maybe I should "shut the fuck up at the same time" but I don't like the way you think about developing this library and I just wrote it. What you will do with it, it's up to you.

@sstok
Copy link

sstok commented Mar 23, 2017

@Wirone I tend to agree with @mnot here.

It might seem like something simple but if you don't have the time to do it and have no interest of using it yourself (because you don't need it) it's only more work to do. I haven't used the GitHub rebase feature, but in the past rebasing some else work required a bit more work.

Even something simple? Yes, even if it's something simple, today it's this, tomorrow something else.
Users will come to expect you will do it anyway (because you did it before), completely ignoring the fact they are working with "human beings" who are doing this for free (no money is made) only doing this to benefit someone else or helping someone because they want to.

While something else they could be doing that would actually benefit them, but instead they are doing something for someone else who may not actually be thankful.

The worst example I have seen was in Sematic-UI, the head maintainer (there is only one) is overrun with work on the project and many understand that. Except one person..., who kept complaining that his "company" decided to use semantic-ui but there were some problems and demanding a good proper solution. Not providing any financial support or anything, no because it's free and making money on someone else work doesn't mean you have to share that 😑

Hintjes (the creator or ZerroMQ) actually wrote a book about this subject.
https://hintjens.gitbooks.io/social-architecture/content/chapter1.html

This works both ways, the author should be willing/able to make the changes when requested, while the maintainer needs to merge directly after approval (without delay).

The fact that the author (no offense) has not responded to the request of rebasing shows a "lack of interest" (for any reason; no time, not used/needed anymore). So why should the maintainer invest time in this if the author doesn't even take the time to respond?

Some might take this personal, but if it happens more then often it only creates noise and irritates people. And you need to take a stand to protect yourself from going crazy or getting into a burnout!
https://www.youtube.com/watch?v=cOjS7iof6SU
https://www.youtube.com/watch?v=wI5AEHPvYms (not sure if this was related, but in this channel there was video about dealing with mental issues)

As Hintjes once said:

If someone wants a feature, they either send us a patch, or offer someone money to make the change, or they wait. This means people only make changes they really need to make.

If you need this feature, propose it. Wait for feedback, and then when changes are requested make them. If the maintainer is not responding, kindly ask what's missing and poke them if things take to long, if they ask for a rebase do it. Remember you need the feature! - Be willing to invest your time.

If the author abandoned the pull-request but you still need the future, open a new pull request with changes. If the maintainer is still unwilling to merge it, ask what's wrong. If nothing is wrong and it's still not merged, then simply give-up.. Like I said, it works both ways.

From what I have seen by older issues the maintainer is not unwilling or resistant to help.
Unless you can provide actual proof about a hateful attitude, this is not really helpful.

Secondly "Sadly, repo owner is probably most user-unfriendly (not to say more) guy I met on GitHub so this little change couldn't be incorporated for almost 4 years."

"most user-unfriendly" 😐🙄 Yea, can understand why someone be pissed of, this was your first comment in this repository. And it shows of no respect or understanding.

Don't assume someone is a horrible person unless you know them personally or have some good proof about there actions (being hateful to others for no good reason), and not doing what you ask (demand) is no reason to call someone "the most user-unfriendly". Otherwise just shut the f**k-up 🙂

@Wirone
Copy link

Wirone commented Mar 23, 2017

@sstok I understand everything you said and mostly agree. Maybe I shouldn't use those exact words, but I was really dissapointed when I saw this is still not merged (I've checked for this few times before). However I didn't demand anything because I've included changes from this PR in our local version so I'm not in need this to be merged - I just said what I think about leaving it for years. There's also slight difference between "the most user-unfriendly" and "probably most user-unfriendly guy I met on GitHub" since it's only my feeling, based on other issues/PRs in this project.

Conclusion: @mnot can do what he wants, I can think what I want, but maybe it could be said in better way.

Cheers.

@sstok
Copy link

sstok commented Mar 23, 2017

However I didn't demand anything because I've included changes from this PR in our local version so I'm not in need this to be merged.

I was just speaking in general here 😉

@gobengo
Copy link
Contributor

gobengo commented Apr 1, 2017

I rebased at made PR as #61

@mnot mnot closed this in 090d353 Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants