| [23:09:58] | * Mo[m] has joined #farmos |
| [23:09:58] | <Mo[m]> | Yeah, it wasn't too hard and using the preprocess hook, it's easy to manipulate. |
| [05:00:07] | * piegeux[m] has quit (Quit: You have been kicked for being idle) |
| [12:18:27] | <mstenta[m]> | jgaehring paul121 symbioquine wotnak ^ |
| [12:18:27] | <mstenta[m]> | > application sent! we'll see if they accept us 🤞 |
| [12:18:27] | <mstenta[m]> | Good news! Netlify accepted farmOS as an open source organization! 🎉 |
| [12:36:38] | <mstenta[m]> | I need to set up the account and add everyone as a user |
| [12:36:57] | <mstenta[m]> | They also sent instructions for transferring projects to the new org if we want to |
| [13:02:22] | <mstenta[m]> | Also: I tried getting 2.0.4 released, but ran into a failing test after updating Drupal core that I need to investigate |
| [13:28:58] | <paul121[m]> | amazing! maybe we could move the farmos-demo site over too - that is a netlify site :-) |
| [13:29:43] | <mstenta[m]> | oh cool! didn't know that :-) |
| [13:30:32] | <mstenta[m]> | one thing that they did note in the email is we are still bound by the limits on traffic etc, and if we go over we gotta pay uo |
| [13:30:36] | <mstenta[m]> | up* |
| [13:31:31] | <mstenta[m]> | maybe worth reviewing the current stats on farmOS.org and the demo site |
| [13:32:02] | <mstenta[m]> | i'm sure it's all within the limits, but it would be good to have a baseline for historical reference |
| [13:34:19] | <paul121[m]> | totally. the demo site does use netlify functions (like aws lambda/"serverless") but I think they are well within the free tier. the actual demos are hosted on a free Tugboat.qa tier |
| [13:35:14] | <mstenta[m]> | ah cool |
| [13:36:56] | <mstenta[m]> | > Also: I tried getting 2.0.4 released, but ran into a failing test after updating Drupal core that I need to investigate... (full message at <https://libera.ems.host/_matrix/media/v3/download/libera.chat/9d62bf4c6a...) |
| [13:37:21] | <paul121[m]> | link to GH action? |
| [13:37:54] | <mstenta[m]> | https://github.com/mstenta/farmOS/actions/runs/4584784653/jobs/8125132853 |
| [13:39:24] | <mstenta[m]> | That is 2.x on my fork, which has two additional commits: |
| [13:39:24] | <mstenta[m]> | 1) https://github.com/mstenta/farmOS/commit/e3d2a621bd43be0d32573da96eaf6ca... |
| [13:39:24] | <mstenta[m]> | 2) https://github.com/mstenta/farmOS/commit/ec27773940dcaa0c04f5df8b4c1263d... |
| [13:39:32] | <mstenta[m]> | 2.x on origin is passing |
| [13:39:54] | <mstenta[m]> | So it probably has something to do with drupal 9.5.7... ? 🤷 |
| [13:40:44] | <mstenta[m]> | Or 9.5.6, actually... 9.5.7 came out right after and just reverts one of the commits in 9.5.6... but most of the changes are in 9.5.6 |
| [13:40:45] | <mstenta[m]> | https://www.drupal.org/project/drupal/releases/9.5.6 |
| [13:50:56] | <paul121[m]> | so doesn't have disabled attribute |
| [13:51:04] | <paul121[m]> | https://github.com/farmOS/farmOS/blob/a56fccaa8f846a9f489028d8839cd01675... |
| [13:56:39] | <mstenta[m]> | yea |
| [13:58:42] | <mstenta[m]> | hmm maybe related to this change? https://www.drupal.org/project/drupal/issues/994360 |
| [13:59:12] | <mstenta[m]> | Definitely has to do with #states and checkboxes in our form... |
| [14:00:51] | <paul121[m]> | Yeah. The last comment in that issue is prob it |
| [14:01:11] | <paul121[m]> | $(e.target).prop('checked', e.value).trigger('change'); |
| [14:01:11] | <paul121[m]> | $(e.target).closest('.js-form-item, .js-form-wrapper').find('input').prop('checked', e.value).trigger('change'); |
| [14:01:20] | <mstenta[m]> | Relevant line in our code: https://github.com/farmOS/farmOS/blob/a56fccaa8f846a9f489028d8839cd01675... |
| [14:01:21] | <paul121[m]> | This code change: https://git.drupalcode.org/project/drupal/-/commit/1c58414870f333dc66b9e... |
| [14:02:23] | <paul121[m]> | ah: https://www.drupal.org/project/drupal/issues/2945727 |
| [14:02:46] | <mstenta[m]> | ah ha |
| [14:02:51] | <paul121[m]> | wow, plain as day: |
| [14:02:51] | <paul121[m]> | > States on radios and checkboxes elements are currently broken until #994360: #states cannot check/uncheck checkboxes elements is resolved, but when that issue is fixed another appears. |
| [14:03:06] | <paul121[m]> | > what that issue is fixed another appears |
| [14:03:10] | <paul121[m]> | s/what/when/ |
| [14:04:42] | <paul121[m]> | ugh so idk how we add those classes to the checkbox wrapper. not sure if we have access to that in our settings form? |
| [14:04:59] | <mstenta[m]> | i can run a quick test with the patch from that issue to see if it works (and still applies) |
| [14:05:32] | <mstenta[m]> | last updated in 2018 for drupal 8.7.x :-/ |
| [14:07:54] | <mstenta[m]> | test started... |
| [14:08:12] | <mstenta[m]> | https://github.com/mstenta/farmOS/actions/runs/4599904179 |
| [14:13:07] | <paul121[m]> | hmm I'm inspecting a settings page now. It seems that the checkboxes do have the js-form-item class in their wrapper |
| [14:13:08] | <paul121[m]> | the issue might be that the "Install modules" button does not |
| [14:14:29] | <paul121[m]> | <paul121[m]> "This code change: https://git...." <- I dunno.. don't fully understand the JS states logic from just looking at this |
| [14:23:41] | <mstenta[m]> | The patch applied but did not fix our test |
| [14:26:46] | <paul121[m]> | wait have the tests completed? |
| [14:28:08] | <mstenta[m]> | Yea, sorry I re-ran failed tests - mariadb failed first, and have BOTH a failure due to the existing mariadb database connection issue BUT ALSO showed the same testInstallFunctionality failure as well |
| [14:29:50] | <paul121[m]> | dang |
| [14:32:49] | <paul121[m]> | Okay, yep |
| [14:37:12] | <paul121[m]> | When a module checkbox is clicked/checked, Drupal states logic should remove the `disabled` property from the "Install modules" button |
| [14:41:41] | <paul121[m]> | But that isn't happening because this change selects the "install modules" button itself (it has the `.js-form-submit` class), and then tries to find child `input` elements, however itself is the `input` element! |
| [14:41:42] | <paul121[m]> | https://git.drupalcode.org/project/drupal/-/commit/1c58414870f333dc66b9e... |
| [14:42:07] | <paul121[m]> | so thus, no child input is found and disabled is never applied |
| [14:43:02] | <paul121[m]> | <paul121[m]> "When a module checkbox is..." <- Subtle detail: the inverse of this should happen when the page is loaded, so that is why `disabled` is never applied in the first place |
| [14:43:03] | <paul121[m]> | * so thus, no child input is found and disabled is never removed |
| [14:45:13] | <paul121[m]> | ..... so I think this would be fixed if we can remove `.js-form-submit` from the "Install modules" input itself. removing a class is harder than adding :-/ |
| [14:49:45] | <paul121[m]> | IMO release without updating Drupal core :-) |
| [14:51:39] | <paul121[m]> | Or, consider this as a pretty minor bug that doesn't have ill-consequences if the user clicks "install modules" without any selected. We could add form validation to give an error message |
| [14:52:17] | <paul121[m]> | (side note - quite cool that our tests caught this!! :P) |
| [15:26:11] | <mstenta[m]> | Should we just remove the disabled state logic altogether from our form? |
| [15:26:27] | <paul121[m]> | thats an option too |
| [15:26:38] | <mstenta[m]> | It's a "nice to have" |
| [15:26:40] | <mstenta[m]> | But not worth spending time fixing this IMO |
| [15:27:06] | <paul121[m]> | Right. So we could remove code and maybe add back in later... or kinda let the bug live on? 🤷 |
| [15:27:36] | <paul121[m]> | I guess we need to address the test error at minimum |
| [15:28:19] | <mstenta[m]> | Yea. I say we just remove the logic, don't disable the submit button, don't test that the submit button is disabled, and call it a day |
| [15:28:46] | <mstenta[m]> | Then someone in the future can try to redo it if they want... and run into this bug and try to fix it in the process |
| [15:30:42] | <mstenta[m]> | Maybe we also add a validateForm() that checks to make sure modules that are not installed were selected |
| [15:32:15] | <paul121[m]> | sounds good to me |
| [15:32:31] | <mstenta[m]> | k i'll throw together a PR right now... |
| [15:33:51] | <mstenta[m]> | > Maybe we also add a validateForm() that checks to make sure modules that are not installed were selected |
| [15:33:51] | <mstenta[m]> | actually, maybe this isn't necessary - submitForm() basically does this and bails early if none were selected |
| [15:35:51] | <mstenta[m]> | https://github.com/mstenta/farmOS/commit/fa45b8bb21943099a9a1ecf4dc30ed9... |
| [15:36:27] | <mstenta[m]> | We don't need to do a full PR for this IMO - if the tests pass, and if you can just take a quick look at that commit paul121 ? |
| [15:51:55] | <paul121[m]> | a PR would actually be nice to find this code later on :-) |
| [15:51:55] | <mstenta[m]> | tests passed |
| [15:51:55] | <paul121[m]> | or update the changelog I guess |
| [15:51:57] | <mstenta[m]> | ok |
| [15:52:35] | <mstenta[m]> | does the commit look good, so i can just make the PR and immediately merge it? |
| [15:53:23] | <mstenta[m]> | (signing off soon - would love to get 2.0.4 released before the end of the day, not sure what your availability to review is) |
| [15:53:23] | <paul121[m]> | Yes, looks good to me |
| [15:53:27] | <mstenta[m]> | cool! |
| [15:53:33] | <mstenta[m]> | i'll wrap it up in a PR as-is |
| [15:53:46] | <mstenta[m]> | thanks1` |
| [15:53:49] | <paul121[m]> | wow didn't realize the time already |
| [15:53:51] | <mstenta[m]> | * thanks! |
| [16:46:11] | <mstenta[m]> | farmOS 2.0.4 has been released! https://github.com/farmOS/farmOS/releases/2.0.4 🎉 |