Closed (fixed)
Project:
Drupal core
Version:
9.0.x-dev
Component:
meetings
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
2 May 2018 at 11:19 UTC
Updated:
21 May 2022 at 15:09 UTC
Jump to comment: Most recent
Moderated By: justafish
Hello! Welcome to to our weekly JavaScript initiative meeting. It happens every Monday right here in Slack!
- This meeting is chat-only. There is no video or audio component.
- We leave about 5 minutes between topics, sometimes more, so that people who are multitasking- The meeting is threaded, so please keep an eye on thread notifications (this may not pop an alert depending on your settings.)
- Please DM me or @drpal if you would like any items added to today’s agenda
1️⃣ Please introduce yourself if you’re attending today’s meeting!
| lauriii | Hey! Lauri @ :flag-th: |
| justafish | Sally in :uk: |
| dawehner | Daniel in :slack: |
| drpal | Matt in :flag-us: |
2️⃣ @lauriii would like to talk about which was almost ready to go and then was thwarted by SVGs!
| drpal | Yooooooooooo |
| drpal | This issue. |
| drpal | I have been totally unable to replicate any of the described issues with SVG insertion. |
| lauriii | for me the current approach seems rather risky, there’s some issues that has been already raised and we don’t really know if there’s something more that is still unknown |
| lauriii | @drpal do you know if there’s a particular reason to go with the current approach? |
| drpal | @lauriii I think the TL;DR of this issue is that any kind of logic to wrap the returned content is immediately more complicated and risky than just inserting what is returned. |
| lauriii | I’m assuming that the root cause for the bug is that in some cases we shouldn’t be adding the wrapping div (but in most cases it doesn’t cause any problems, and can be reasonable) |
| drpal | @lauriii If you ever end up with content that returns another link that is ajax-enabled, then you can possibly end up in a situation where you’re inserting a cascading series of wrapping divs. |
| drpal | Back in #246, |
| drpal | There are some examples of the unreasonably complicated logic. |
| drpal | I don’t think we should be making any assumptions about the returned content, and if it should be wrapped. |
| drpal | If someone using this system, wants to ensure their content, whatever that may be, is correctly displayed, it should be on them to provide the correct wrapping elements if needed. |
| drpal | Additionally, about the actual SVG problem. |
| drpal | I haven’t been able to replicate at all. |
| lauriii | could we scope the issue in a way that we wouldn’t have to provide that logic? Or is the cascading series of wrapping divs a major consideration for this issue? |
| lauriii | if this indeed is the best approach, maybe we should look into implementation that wouldn’t break the BC |
| drpal | @lauriii Originally, I think it was in the original issue summary, that sometimes non block level elements were being wrapped when they shouldn’t. |
| drpal | So in addition to a cascade of wrapping divs, we could be inserting incorrect HTML markup. |
| lauriii | yeah that was the issue that I saw |
| tim.plunkett | Right |
| drpal | @tim.plunkett :calculator: |
| drpal | _shit_ |
| drpal | Should have been, :clap: |
| lauriii | Have we explored if we could make the wrapping element configurable, and no wrapping div being one of the options? |
| tim.plunkett | Callers of this don't necessarily know what the contents will be, nor how/if to wrap it |
| drpal | Exactly. |
| drpal | Not wrapping the response is, _IMHO_, the safest way to handle this. |
| lauriii | thanks for the pointers, it indeed seems like the best approach |
| lauriii | I think this function shouldn’t conceptually be wrapping its content with divs so the change makes sense |
| drpal | @lauriii Also in #312, , I tried to provide some reasoning about why we have to use `$.parseHTML()` and `$()` |
| lauriii | however I’m still concerned, how do we make sure that this doesn’t break something |
| lauriii | it seems like the SVG issue might be still valid |
| drpal | @lauriii Have you been able to replicate it? |
| lauriii | it seems like a jQuery bug, if you cannot reproduce the bug, could you be running different version? |
| lauriii | I found this jsfiddle online |
| drpal | I applied the test case patch onto the 8.6.x head. |
| lauriii | which is proving that this bug does exist in jQuery |
| drpal | @lauriii I don’t think this really highlights the issue. |
| drpal | It’s also jQuery 2.x |
| drpal | And we are on 3.x |
| lauriii | I can reproduce the bug with the instructions on the issue |
| lauriii | exactly same behaviour as on the jsfiddle |
| drpal | @lauriii Strange. In both gif’s I posted, I didn’t get any of that behavior. |
| drpal | However, this doesn’t actually look like an issue we can solve. |
| lauriii | @drpal did you apply both, the patch they provided and the patch with the change? |
| drpal | @lauriii are you talking about #316? |
| lauriii | yup |
| lauriii | I applied that + #296 |
| drpal | Yep. I am an idiot and didn’t apply 296. |
| drpal | I forgot I wasn’t on my test branch for this. |
| drpal | Ugh. |
| drpal | @lauriii did you see this discussion? |
| lauriii | interesting |
3️⃣ is still moving along, not sure if you have any comments on that one @drpal?
| drpal | The patch in #41 is :100: |
| drpal | In the previous 1/2 no-restricted-syntax patch there was a minor regression. |
| dawehner | This logic still confused me the hack out |
| dawehner | ```If (...every()) { return false}``` |
| dawehner | This is the logic afterwards |
| dawehner | Previously |
| dawehner | ```if (result === false) { return false; }``` |
| drpal | @dawehner Yeah. Sorry it wasn’t really possible to simplify this even further. |
| drpal | Perhaps we can have a followup to address the bigger picture with our early-exit loop obsession? |
| dawehner | @drpal Good idea! |
| drpal | @dawehner YAYAYAYA! |
| dawehner | @drpal Let me clarify why I'm confused. There is no return true in the ```.every()``` function |
| drpal | JavaScript: You didn’t return true. Me: I know. JavaScript: So you just want me to assume that it’s always true? Me: Sure. |
| drpal | @dawehner But I understand the confusion. |
| dawehner | @drpal well, I think we should still do an explicit return, it makes it easier to understand what is going on, don't you think so? |
| drpal | @dawehner Yes. I can upload a patch and :pray: that it works? |
| drpal | Maybe later today. |
| dawehner | @drpal thank you! |
| dawehner | @drpal That removes some confusion for me at least |
| dawehner | @drpal I'm happy to patch that out as well |
| drpal | @dawehner If you want, I can review? |
| drpal | Whatever you have time for. |
| dawehner | @drpal Well, I have a trip in the tube in front of me. |
4️⃣ finally committed! :tada: I’d love to draw attention to @webchick’s comment here as it’s _v good_
| dawehner | Oh yeah :100:. There is no direct value in using react. The Value is is being able to provide a more easy to use interface on the longrun and maybe move Drupal towards a better architecture. |
5️⃣ droplet is stepping down as a core JS maintainer, the issue is here if you’d like to show your thanks and appreciation for all his hard work over the years
| tim.plunkett | :clap: :pray: :blue_heart: |
| dawehner | Why is there no JS emoji yet? |
| drpal | :javascript: |
| drpal | `:javascript:` |
| dawehner | OH |
| dawehner | I tried :ecmascript: and it didn't work |
| drpal | _rlol_ |
| justafish | lol Daniel |
6️⃣ We won’t be giving an initiative update/discussion at DrupalCon, should we do a BOF?
| drpal | Yes. |
| eatings | Yes |
| eatings | It would easily fill a room and probably could extend well beyond the 1 hour block |
| lauriii | Yed |
| lauriii | Yes |
| dawehner | Yes, at this point a bof feels more productive anyway |
| eatings | you could also make sure it’s written on the BoF board _mit Frakturschiften_ so you get to keep your blackletter type |
| dawehner | It should totally get the same treatment as in: Have some planning, bulletpoints to talk abobut |
| dawehner | There should be a fire emoji-unicode on the board |
| drpal | @dawehner I’m bringing the giant set of balloons. |
| drpal | So people will actually be able to find us. |
| eatings | actually |
| eatings | the best way to to get people to come to this meeting is to leak on twitter that there will be a Big Javascript Decision made at a meeting |
| dawehner | lol |
| eatings | we will need to hire bouncers if that happens |
| drpal | Yeah, we are going to teach everyone that it’s _JavaScript_. |
| eatings | dogg, i’m not even going to start |
| dawehner | @eatings And then we talk about semicolons or not |
| eatings | about capitali[s]ations |
| drpal | I’ll give away all the 40" balloons I got that spell out JavaScript |
| justafish | lol @eatings that is actually a great idea |
| eatings | BizDave is definitely an Idea Guy™, @justafish |
| eatings | i’m all for either going huge big with the balloons for the prominence or the deliberately sly ‘psst did you hear about the secret JS meeting?’ route |
| drpal | well considering i already bought the balloons, we are using them either way. |
| eatings | oh |
| eatings | well then |
| justafish | @drpal do you have peace wave balloons too? |
| drpal | No. |
| eatings | i still think we should leak on twitter in the Most Casual Way possible that there will be a Big Javascript Meeting |
| drpal | I just bought the letter ones. |
| eatings | you know, you could’ve just saved a ton of money by simply buying J and S :stuck_out_tongue: |
| eatings | another free Big Idea™ from BizDave |
| justafish | lol |
| justafish | ok so do we want to run this with the same format as if it were a core convo? small presentation and then discussion |
| drpal | :thumbsup: |
| dawehner | :+1: |
| eatings | we should also all be wearing robes and have candles arranged in a circle in the middle of the room, in the manner of a secret gnostic ceremony |
| eatings | chanting in old german or latin |
Comments
Comment #7
justafishComment #8
justafishComment #10
justafish