published @ https://www.drupal.org/project/entity_browser_block/releases/2.0.0 →
there are some parts in the diff I might look into later but for now LGTM and tests pass
thank you!
SGTM, added
mcdruid → credited samuel.mortenson → .
Due to #76 I'm honestly not sure how to "properly" build a Docker image that works with Drupal 11 and SQLite.
Oh, with CKEditor4 gone this is much harder than I thought
samuel.mortenson → created an issue.
When I wrote Tome (a static site generator) I wanted the performance to be better than `wget --recursive`, so I added in the ability to handle multiple HTTP requests in one bootstrap. There are still many odd bugs, but here are some overrides I did to reset "state" between each request:
https://git.drupalcode.org/project/tome/-/blob/8.x-1.x/modules/tome_stat...
Might be helpful to future core changes!
MR is at https://git.drupalcode.org/project/tome/-/merge_requests/29, ignore the issue fork I truly loathe the current contribution experience and do not know how to link existing MRs to issues.
samuel.mortenson → created an issue.
Created https://git.drupalcode.org/project/moderation_sidebar/-/merge_requests/23 I have no idea how MRs work still
The CSS changes don't work with D10. Specifically, the ".ui-dialog-off-canvas #drupal-off-canvas-wrapper" selector doesn't work, "#drupal-off-canvas-wrapper" is the same element as ".ui-dialog-off-canvas". I'm not sure if this patch will mess up specificity or something but it works in D10 for me. See screenshots for what I saw on stock d10 install and 8x-1.8.
Official docker image having the same issue: https://github.com/docker-library/drupal/issues/264
I'm finding this pretty inconvenient, I use sqlite for all my local development and it's the default database for Tome projects. That means myself and all Tome users (of which there aren't thousands, to be fair) will have to upgrade sqlite3, which for Ubuntu users means an upgrade to 24.04 or building from source. All this for a MR that isn't in RTBC and doesn't use JSONB feels odd to me.
"First I have enabled automatic mode"
Could you upload your highlight_php configuration here? I'd like to see what your configured languages are. It only chooses from languages in config to avoid false positives, so if ruby has a high priority there and plaintext isn't in the list, that could be what's happening.
"I have enabled manual mode, but the code is not highlighted."
You code samples here look OK to me - I think "cs" is the code for "C#", which is why it's not highlighting as bash (sh).
Actually I think I just realized what you were saying, sorry about that.
I saw this too - but the page isn't actually blank (you can verify this with curl), it's just that your browser/web server can't recognize the content type of the file. Since Tome doesn't control how files are served, this issue isn't fixable by Tome AFAIK.
Sorry for the delay and thanks all - I've applied the patch in #7 in addition to doing a few other minor fixes. Should release later today.
I think that makes more sense - if you can't edit something make a flag/note. If you can edit, just make the change yourself. Kind of an enforced DIY thing for that class of users? I don't think this is a common use case, but can understand it now.
Your changes in PR look good too so when tests pass I can approve (or you can assume that you're good to commit).
I think more permissions are probably good, but I don't get the business use case of this one:
Able to create top level notes, only if the user can not edit the content. If the user can edit the content, they should fix the issue instead of leaving a note
Would users with this permission intuitively know why they can't leave a note on some entities but not all entities they have read access to? Maybe if you explain the use case more it'll make more sense to me. Thanks for the PR!
Thanks for the work all - putting back to "Needs work" for two reasons:
1. Drush versions prior to 12 allowed Symfony commands defined as services to be executed. This isn't a Drupal 10 issue or a Tome issue (subjectively, I had been told that this had worked in Slack but it's undocumented), it's a Drush 12 issue. If we can figure out the issue with Drush 12 and make a PR to the Drush repo that would be preferable.
2. If (1) isn't possible, we can do #10, but that feels like a breaking change to me. I don't want to make a new branch/minor and also don't feel like adding Drush as a composer dependency is appropriate. Need to think about this more.
Hi @ivnish, thanks for the offer! I just added you as a co-maintainer to the project.
samuel.mortenson → created an issue.
@bluegeek9 Yeah I think that would be OK
@akshaydalvi212 Did you manually test this? I'm not sure I want markdown rendering in hook_help (unless that's available in core), but there was a chunk of code that converted .txt formatting to HTML which isn't removed in your MR.
@bluegeek9 sorry for responding via email - regarding project maintainership, I usually ask people this:
Would you be able to spend some time in the issue queue reviewing patches and submitting one new patch for review? I typically ask this of people seeing co-maintainership as a way to understand their working style.
For example, I noticed this issue is RTBC but tests are failing: https://www.drupal.org/project/moderation_note/issues/3329374 📌 Automated Drupal 10 compatibility fixes Fixed
This is RTBC but would have broken hook_help: https://www.drupal.org/project/moderation_note/issues/3334422 📌 Replace README.txt with README.md Fixed
So maybe a new issue+patch to fix tests would be a good place to start? Then Drupal 10 compatibility. I'll try to be more responsive via email too.
Closing this as I don't plan to transfer maintainership right now, but would add you as a comaintainer after reviewing a patch you wrote.
The .txt version is used for auto-generating help text for the module.
samuel.mortenson → created an issue.
samuel.mortenson → created an issue.
We discussed this in a private chat already, but I don't like the idea of having a typed data normalizer that normalizes \Drupal\layout_builder\Plugin\DataType\SectionData, but does not return an instance of SectionData when denormalized. Normalizers are meant to accept their ::supportedInterfaceOrClass in normalize, and return an object of that class in denormalize based on the $class argument. Changing this behavior defeats the purpose of normalization for me, even if core is already doing this.
Maybe a middle ground would be to not have a typed data normalizer for SectionData, and instead write one for Drupal\layout_builder\Section, which is the value it accepts. Then the field normalizer could use this to set a value on its typed data object.
Fixed the namespace issue, thanks @tim.plunkett!
Missed one.
And updated a test method to actually test ::supportsDenormalization
Fixed coding standards on the new test.
Addressed all points in #4 (including new unit test coverage 🏄♂️) except the validation and schema feedback. May need @tim.plunkett to chime in here for direction.
Here's a start to this, still need to add tests and think about validation.
I opted to write two normalizers - one for the layout_section DataType and another for the field item. This made sense to me as it allows for new kinds of layout field, but we could put everything in the field item normalizer if that seem simpler. I tested GET/PATCH and it seems to work, so feel free to test things out and find bugs as I (or others) write test coverage.
samuel.mortenson → created an issue. See original summary → .