Skip to content

Instantly share code, notes, and snippets.

@p
Created November 20, 2011 04:11
Show Gist options
  • Save p/7dd3fe1ec3a4a1075c74 to your computer and use it in GitHub Desktop.
Save p/7dd3fe1ec3a4a1075c74 to your computer and use it in GitHub Desktop.
(13:18:07) nn-: callumacrae: what is the meaning of data-ajax=true?
(13:18:30) callumacrae: It means it uses AJAX but doesn't require a callback
(13:19:11) nn-: which ajax would be used?
(13:19:17) callumacrae: What?
(13:19:26) nn-: if i write
(13:19:36) nn-: <div id='foo' data-ajax='true'></div>
(13:19:43) callumacrae: Wouldn't do anything
(13:19:52) callumacrae: Has to be an anchor or a form
(13:20:25) nn-: what would happen if div was an anchor above?
(13:20:33) callumacrae: What?
(13:20:49) nn-: <a id=foo href=/ data-ajax=true></a>
(13:21:29) callumacrae: It would send the request to / and see what it sent back, if it sent back some confirm_box data it would display a confirmation box, or it would display an alert saying what the server sent back
(13:23:03) nn-: ok
(13:23:11) nn-: do we have links using confirmation boxes?
(13:23:19) callumacrae: tonnes
(13:23:31) callumacrae: first example that comes into my head is post deletion
(13:24:06) nn-: and what would be an example of us displaying said alert and not changing anything on the page?
(13:25:08) callumacrae: clearing board cookies
(13:25:15) callumacrae: That refreshes instead
(13:26:01) nn-: that is pretty good
(13:30:41) nn-: how much of the diff is needed to do this part?
(13:37:05) callumacrae: just phpBB/assets/javascript/core.js and a few lines of overall_footer.html and prosilver/template/ajax.js
(13:37:29) callumacrae: oh and all the json stuff
(13:37:55) callumacrae: and the functions.php edits
(13:41:00) nn-: ok
(13:41:30) nn-: how do actions that result in changed markup work?
(13:48:42) callumacrae: what do you mean?
(13:50:10) nn-: data-ajax=true only shows server response
(13:50:27) nn-: supposing i delete my post
(13:50:30) callumacrae: Yes
(13:50:31) nn-: page contents needs to change
(13:50:40) nn-: therefore data-ajax=true cannot be used
(13:50:58) callumacrae: They use data-ajax="callback_name"
(13:51:13) callumacrae: in this case, it is called post_delete
(13:51:19) callumacrae: and is added on line 5 of template/ajax.js
(13:51:21) nn-: what is callback_name?
(13:51:40) callumacrae: That was an example
(13:52:16) nn-: yes
(13:52:26) nn-: right now i am a user trying to figure out how to ajax something
(13:52:29) nn-: and i know nothing
(13:52:36) nn-: you told me i need to use data-ajax="callback_name"
(13:52:48) nn-: is callback_name something that is already in the code?
(13:54:28) callumacrae: I'll write a wiki page before release
(13:54:38) callumacrae: but most of it is in the .ajaxify article
(13:54:40) callumacrae: http://wiki.phpbb.com/JavaScript_Function.phpbb.ajaxify
(13:54:56) nn-: i read it again just now and it is not usable as user documentation
(13:55:12) callumacrae: hense why I'm going to write another wiki page
(13:55:29) nn-: go back to the data-ajax=true part and compare it to ajaxify wiki page
(13:56:09) nn-: right now i still don't understand the rest of your diff then
(13:56:25) callumacrae: there are about 300 lines of code
(13:56:34) callumacrae: and then 700 other lines implimenting those 300 lines
(13:58:00) nn-: what are you trying to say?
(13:58:32) nn-: i understand the data-ajax=true part
(13:58:44) nn-: if you can split it from the diff it may be mergeable already
(13:58:51) nn-: the rest i don't understand
(13:58:54) callumacrae: Of course I can't split it
(13:59:13) callumacrae: because it would have to be entirely rewritten in the next PR
(13:59:52) nn-: i can't see this being the case
(14:00:59) callumacrae: this pretty much sums up the entire diff for developers: http://pastebin.com/5rC9RMym
(14:02:11) nn-: when you don't justify your remarks like the one immediately above and i don't see where you get them, you effectively are saying nothing
(14:02:25) callumacrae: yeah you've said that before
(14:02:36) nn-: you keep doing it
(14:02:36) callumacrae: But you're saying "I don't understand" and not saying what you don't understand
(14:03:31) nn-: http://pastebin.com/5rC9RMym - does not answer my question of how to handle changing marking when i delete a post
(14:03:48) callumacrae: what do you mean, changing marking?
(14:03:52) nn-: markup
(14:04:03) callumacrae: line 8
(14:04:17) callumacrae: this.innerHTML, $(this).append(), $(this).remove()
(14:04:19) nn-: nope still don't get it
(14:04:27) callumacrae: "manipulated" means "changed"
(14:06:09) nn-: ok
(14:06:21) nn-: first, that code needs to be converted into prose
(14:06:27) callumacrae: prose?
(14:06:31) nn-: something one reads
(14:06:36) nn-: text
(14:06:42) callumacrae: I know, I said that I would write a wiki page
(14:06:43) callumacrae: lol
(14:07:04) nn-: that is a prerequisite for merging
(14:07:12) callumacrae: k I'll do it now
(14:07:25) nn-: second, in case of post deletion that is insufficient because i cannot just delete the post
(14:07:54) nn-: if the deleted post is in the middle of a topic the page i am on might need a new post brought onto it from the subsequent page
(14:07:55) callumacrae: $(this).parent('div .post').remove()
(14:08:18) callumacrae: It doesn't do that
(14:08:29) callumacrae: I think it refreshes, but I can't remember
(14:08:32) callumacrae: wrote it ages ago
(14:09:42) callumacrae: No, it only refreshes if the server requests a refresh
(14:09:52) nn-: to summarize we now have 3 cases
(14:09:54) callumacrae: eg, first post of thread
(14:10:07) nn-: 1. no changes to markup are needed - send request to server and display response in an alert box
(14:10:38) nn-: 2. changes to markup are needed but all information is already present on the page - server returns status only, page already has all markup and js to do whatever is needed
(14:10:59) nickvergessen: bantu: nothing should be selected after the lastest change i made
(14:11:04) nn-: 3. changes to markup are needed and the page does not have all of needed markup - server must return additional markup
(14:11:14) nn-: 3a. what if there is not only missing markup but also missing js?
(14:11:39) callumacrae: HTML shouldn't be sent over AJAX
(14:11:49) callumacrae: info should be sent and it should be generated by the js
(14:11:55) callumacrae: That would be a huge pain for style authors
(14:12:04) callumacrae: So it would be better to just refresh the page or leave it as-is
(14:12:31) nn-: there you are making unjustified assertions again
(14:12:56) nn-: i live in rails world and we send markup all the time
(14:13:03) nn-: maybe it can't be done, but you need to explain why
(14:13:32) callumacrae: It's just bad practice
(14:13:49) nn-: you just did it again
(14:14:04) nickvergessen: bantu: https://github.com/phpbb/phpbb3/pull/448#issuecomment-2800353
(14:14:28) callumacrae: I don't know why, the people who told me off when I used to do it didn't explain it, but they knew a hell of a lot more than me so I took their words for it
(14:15:09) nickvergessen left the room (quit: Read error: Connection reset by peer).
(14:15:29) nickvergessen [~unknown@dslb-094-218-232-205.pools.arcor-ip.net] entered the room.
(14:15:33) nickvergessen left the room (quit: Changing host).
(14:15:33) nickvergessen [~unknown@phpbb/developer/nickvergessen] entered the room.
(14:15:33) mode (+o nickvergessen) by ChanServ
(14:15:52) nickvergessen left the room (quit: Client Quit).
(14:16:06) nickvergessen [~unknown@phpbb/developer/nickvergessen] entered the room.
(14:16:06) mode (+o nickvergessen) by ChanServ
(14:16:28) nn-: the problem statement is #3 before the dash
(14:16:37) nn-: 3. changes to markup are needed and the page does not have all of needed markup
(14:16:54) callumacrae: changes to markup are easy
(14:17:05) nickvergessen: !irclog
(14:17:06) DavidIQ|Server: http://www.davidiq.net/irclog/default.aspx
(14:17:40) callumacrae: getting markup from the server would be trickier and would involve massive changes to the template system, so it would be easier to refresh the page
(14:18:25) nn-: you can certainly take that approach
(14:20:39) nn-: so those are the 3 sections in user documentation
(14:20:44) nn-: each should have an example
(14:21:27) nn-: and written toward users of that code (i.e. extension authors)
(14:22:32) nn-: at this time it is my impression that as a user i never need to call ajaxify at all
(14:22:47) callumacrae: There are extremely rare cases
(14:22:55) callumacrae: like the quick-mod tools
(14:23:00) callumacrae: Not all of the functions can be AJAXifies
(14:23:03) callumacrae: AJAXified*
(14:23:34) callumacrae: Anyone who is doing something like that will need to read the .ajaxify page though, as it is pretty complicated
(14:23:44) nn-: quick mod tools is a select
(14:23:49) callumacrae: exactly
(14:23:55) nn-: have you considered ajaxifying on select options?
(14:24:26) callumacrae: I've considered having ajax overrides on select options, but it would be too resource intensive to do with every ajaxify
(14:24:35) callumacrae: The amount of cases where it would apply is *tiny*
(14:25:09) nn-: explain resource intensive
(14:26:03) callumacrae: It would have to navigate through the DOM looking for selects, and if it found one it would then have to navigate through the options looking for overrides
(14:26:33) nn-: and what happens now?
(14:27:00) callumacrae: https://github.com/phpbb/phpbb3/pull/336/files#L34R57
(14:29:22) nn-: what happens now for ajaxifying the other elements?
(14:31:38) callumacrae: that doesn't affect other elements
(14:33:43) nn-: that is true but does not answer my question
(14:34:21) callumacrae: In that case, I don't understand what the question was asking
(14:35:38) nn-: (14:24:26) callumacrae: I've considered having ajax overrides on select options, but it would be too resource intensive to do with every ajaxify
(14:36:07) nn-: my hypothesis is ajaxifying links and forms will take the same time as ajaxifying links, forms and select options
(14:36:09) callumacrae: I'm confused
(14:36:38) callumacrae: You can't ajaxify select options because no event is fired on them when the form is submitted
(14:37:22) nn-: we are considering a user's perspective here
(14:37:29) nn-: as a user i ajaxify select options
(14:37:48) nn-: what element that actually sets events on is something i entirely do not care for
(14:38:45) callumacrae: It would be better to have AJAX overrides on the selects, and check the currently selected option for it
(14:39:01) nn-: there you are again with "better"
(14:39:08) callumacrae: So the form has data-ajax="true" and the option has data-ajax-override="true" or data-ajax="false"
(14:39:13) nn-: it would be better to have a consistent api
(14:39:50) nn-: sorry never mind
(14:39:54) callumacrae: I'd love to hear your suggestions
(14:40:49) callumacrae: I asked in this channel how it should be done before I started writing
(14:40:54) callumacrae: Nobody suggested anything
(14:41:05) nn-: at this point it seems like it is possible to avoid calling ajaxify for select options
(14:41:28) callumacrae: With exceptions, I was attempting to make a highly-specific case not so specific
(14:41:33) nn-: and if so i would like to see that option in code
(14:41:39) callumacrae: the solutions to a highly-specific case*
(14:41:42) nn-: or that version
(14:42:12) callumacrae: I don't really understand why
(14:42:24) nn-: because you will go from 2 apis to 1 api
(14:42:56) callumacrae: 2 simple APIs to 1 complicated, conviluted API
(14:43:01) callumacrae: I don't see the advantage
(14:43:15) nn-: ajaxify is not simple
(14:43:36) nn-: i think i still don't understand it
(14:43:43) callumacrae: No, you don't
(14:44:01) nn-: this means you do not have 2 simple apis
(14:44:08) nn-: you have one simple api and one complicated api
(14:44:54) callumacrae: exception is currently only used in one place, but it has potential to be used in other places. Adding additional data attributes would solve it in only one place and make it not useful anywhere else
(14:49:01) nn-: https://github.com/phpbb/phpbb3/pull/336/files#L34R46 does a full dom walk
(14:49:16) nn-: at which point checking select options is free
(14:49:36) nn-: if you restricted the selector to a, form there may be some argument
(14:49:44) callumacrae: If you can impliment it there, I would be amazed
(14:52:22) nn-: the other open issues are whether js should be in .js files or in templates
(14:52:38) callumacrae: ...are you joking?
(14:52:51) nn-: i'm leaning more toward js files now but inheritance needs to be considered
(14:53:42) nn-: and how extensions are going to work with everything
(14:54:55) nn-: no, not really
(14:55:12) nn-: two lines of js which are useless without corresponding markup may be in markup
(14:55:44) nn-: now that js is becoming more general it makes more sense to have it be separate
(14:56:26) nn-: especially stuff like that ajaxify call that is used in a single select
(14:57:36) callumacrae: what's wrong with useless, cached js?
(14:57:56) nn-: it is in a different place from code
(14:58:03) nn-: from markup that it is tied to
(14:59:37) callumacrae: ...?
(14:59:45) nn-: no need to argue about it now
(15:00:23) nn-: if you split the diff
(15:00:40) nn-: #1 would be fairly straightforward
(15:00:48) callumacrae: That's out of the question, I don't have enough time anymore
(15:00:52) nn-: #1 to #2 should be fairly small
(15:00:59) nn-: #2 to #3 would be a mess
(15:01:13) nn-: the mess is an indicator that something is not right
(15:01:47) nn-: #2 to #3 with data-ajax on select options would be a lot clearer in terms of usage
(15:02:01) nn-: you would have significant changes in the implementation but close to none in the api
(15:03:16) nn-: and then just call sites
(15:03:57) nn-: as it is you have those 3 points plus call sites folded into a single gigantic diff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment