Well, this issue is closed, but I would really like to get this working in 10.3.10 and I don't want to have any phantom core update hooks lying around. I considered creating a local patch for the schema definition and manually setting the length in the DB but that seems unwise.
So given the timing, I'll settle for a 10.4 release. I'm not really sure how to proceed with a fork in this scenario so I regenerated a patch using the API suggestion in #71. I think it will also require patching core 11.x (and 11.1.x?) with this:
$equivalent_update = \Drupal::service('update.update_hook_registry')->getEquivalentUpdate();
if ($equivalent_update instanceof \Drupal\Core\Update\EquivalentUpdate) {
return $equivalent_update->toSkipMessage();
}
Is this patch helpful or do we need a core maintainer/git ninja to backport system.install
?
I am not sure whether I should create a new issue for my situation. I am seeing all of the key errors in this issue description resolved by the 1.4.0-rc1 upgrade. So, in a sense, this could probably be closed and marked as a duplicate of https://www.drupal.org/node/3445834 β .
However, I after upgrading to 1.4.0-rc1, I am still seeing this error:
Error message
'replace_empty' is not a supported key.
Nice! I've been using the change on prod. I think we can update the issue status.
I put the patch from comment #4 into a MR.
The patch in #2 changes the way that adminimal admin toolbar has previously been styled in a jarring way by allowing a light background color scheme pass through on the active top level menu items.
I've attached a patch that restores the styling to match previous dark background version of the adminimal admin toolbar.
amanire β made their first commit to this issueβs fork.
Marking as a duplicate of https://www.drupal.org/project/yoast_seo/issues/3362165 π Fix Deprecated function: Creation of dynamic property Drupal\yoast_seo\YoastSeoManager::$yoast_seo_field_manager is deprecated RTBC which is older and RTBC.
Just to clarify my comment in #6. I meant that it should at least be configurable if the default was not least destructive. But if the default is changed, I don't care if it's not configurable. Thank you for moving this forward!
Just noting that I have a different use case where I'm seeing this issue occur. In my case, it's because we have organized the node title inside of a field group widget on the form display. It has the same effect on the layout as long strings. I'm not sure if it's worth adding that to the title and/or description though.
Thanks @simohell!
Looks like it was reverted as part of https://www.drupal.org/project/drupal/issues/2519362 π Redesign the menu link add form to be less overwhelming Fixed .
I'm testing a Drupal 10.3.0 upgrade and seeing this issue again. It looks like a regression.
Noting that the workaround in #14 also resolved this for me. But that it took me a while to figure out how to reproduce and confirm because I was not creating watchdog entries with the warning when logged in with the "Administrator" role. Once I realized that this was happening, I tried logging in with another user account with fewer permissions and successfully reproduced the warnings in the description.
Marking as Fixed since this is in the 2.0.2 release.
amanire β created an issue.
Good to see progress here! FWIW I just deployed the patch in #48 to a production site and it resolved the deprecation warnings described in https://www.drupal.org/project/profile/issues/3324465 π [PHP 8.1] Deprecated function: unserialize(): Passing null to parameter #1 ($data) of type string is deprecated in Drupal\Core\Entity\Sql\SqlContentEntityStorage->mapFromStorageRecords() Closed: duplicate . I was a little surprised at the substantial CPU performance improvement for the site, presumably because these were invalidating the varnish cache.
I don't understand the comment in #6. I see the ' $input.attr is not a function' error after applying the MR!34 patch. The patch in https://www.drupal.org/project/context/issues/3374119 π Search is broken when placing block RTBC resolves the issue for me.
MR!36 works great, thank you!
But this issue is more widespread than the description here and in https://www.drupal.org/project/context/issues/3355665 π Uncaught TypeError: $(...).once is not a function RTBC indicate. It took me a few tries to track this issue down because of the title. I'm revising it and description for clarity to help others in this situation and better expose its severity to the module maintainers.
Sorry that wasn't clear. My site's custom theme. I think with core, it would just result in <div></div>
. In my particular case, we have an additional div in the block template which monkeyed with the spacing.
This is a great little module. An important UI feature that I would like is a way to filter the list of contexts. Some of the contexts are created for other purposes and are irrelevant and inappropriate to use within layout builder. I'd like to minimize any confusion for our content admins. Layout builder is confusing enough as it is! It could be an option on a per context basis but I like the idea of associating it with one or more context groups.
I hope this isn't forgotten. The commit is an improvement but results in a couple of nested empty divs, which were visible in my theme. I'm not sure what a better solution is though. It kind of seems like a core bug to assume the existence of the content
key? As a workaround, I added this to the theme's theme_preprocess_block()
:
if (isset($variables['content']['#access']) && $variables['content']['#access'] === false) {
$variables['attributes']['class'][] = 'hidden';
}
I'm looking at the code for 8.x-1.x and the patch in #8 has not been merged. This issue is not fixed and should not have been closed. It recently appeared on a project that I'm working on with version 8.x-1.5. I can't reopen this since I'm not a maintainer, so I'm debating whether I should create another issue that references this one.
Like @markconroy in #9, I am also seeing this on the body field of a node field widget (without paragraphs) when the "Apply filters" exposed filter is submitted via AJAX. I am also seeing this when media library is accessed via the Drupal Media button in CKEditor (in both 4 and 5). The change in cascade order of .ui-widget
from /core/themes/claro/css/components/jquery.ui/theme.css
overriding .button--primary
from /core/themes/claro/css/components/button.css
is definitely causing the display issue.
This seems related to https://www.drupal.org/project/drupal/issues/3378341 π claro.jquery.ui css assets may be added the page multiple times Fixed and may be an issue with Claro? I'm not sure whether I should rewrite this issue title and description without paragraphs and seven or create a new issue entirely.
This issue is a blocker for me. I would also like to know if this is ready for testing.
https://www.drupal.org/project/media_entity_file_redirect β was listed twice in the issue description.
I just ran into this error again. In case others find this, I've been successfully using this drush script to remove invalid permissions without patching core:
https://www.drupal.org/node/3193348#comment-14969643 β
OK this patch now includes the explicit accessCheck for D10 compatibility.
I've updated the patch in #80 for Drupal 10 compatibility. The corresponding feed had stopped importing and was throwing the following warning:
Entity queries must explicitly set whether the query should be access checked or not. See Drupal\Core\Entity\Query\QueryInterface::accessCheck().
I just tested the latest change in the MR and it works great for me at wide (1920px) and narrow (977px) viewport widths in MacOS Brave (Chromium), Firefox and Safari. Combine that with the feedback in https://www.drupal.org/project/drupal/issues/3381219#comment-15321780 π Layout Issues with Claro theme Needs work and I think we can update the status to RTBC. Nice work @simohell!
I'm having second thoughts about setting a max-width on the node form. One thing I noticed is that other admin forms do not have a max-width set for wider screens (for example, /block/add
or /admin/structure/types/add
). I agree with the typographic principle of keeping line length < ~75 characters but it also seems maybe that itβs inconsistent with the general approach of the Claro theme to use the available space. Also, I expect that the lack of a max-width would be preferred on larger screens for more complex UI elements like vertical tabs.
Ah, sorry, I described that incorrectly @simohell. The problem with the horizontal scrolling on narrower viewports is caused by grid-template-columns
. That issue is noted and addressed in this patch and comment:
https://www.drupal.org/project/drupal/issues/3381219#comment-15238921
π
Layout Issues with Claro theme
Needs work
Ok. I updated the CSS so, that this fix does not create regression to Firefox and Safari.
If you are referring to my comment in #19 π Long string breaks the layout of Claro Fixed , I am testing in Brave 1.60.114 Chromium: 119.0.6045.124, and am still seeing the same horizontal scrollbar in other browsers. I was not describing a cross-browser issue, but a hard-coded width issue that results in scrolling on any browser viewport width between 976px and 1278px wide.
@simohell yes, I agree about limiting the character width on larger screens. That is also consistent with prior float-based layout so I would personally consider removing it a regression. I think this MR needs to be cleaned up a little, but I just set max-width: 76rem
on node-form-layout
, which results in the same main column max width as it was previously (52 rem) and restored the margin-inline: auto
to center it. This eliminates that regression on larger screens.
The Title text field is set to a size of 200 in these screenshots. I'm seeing the same results in MacOS Brave (Chromium), Firefox and Safari.
Display on 1920px wide screen
Display at 977px wide viewport
Thank you for referencing that issue @simohell. I tested the work there locally and found that it did address the layout issue but introduced a regression in the responsive layout at viewport widths between 976 and 1278px. I posted screenshots in a comment there showing the difference π Long string breaks the layout of Claro Fixed .
I've been just comparing the solution in this MR against the work in #3381219 π Layout Issues with Claro theme Needs work and while it does resolve the issue with the main column expanding, it also introduces a fixed width when the browser viewport is between 976px and 1278px wide. On narrower viewports, the user must scroll to the right to view and edit the secondary column. I think we should consider this a regression, since the prior float-based layout was fully responsive and compressed the main column. I've confirmed that the scrolling appears in multiple browser engines.
I don't know, maybe I'm biased. But I prefer this approach from #3381219 π Layout Issues with Claro theme Needs work
Updating the issue tag. The issue is with CSS grid, not flexbox.
And updating the title to clarify that this is specific to the node form layout.
I was referred to this issue by way of https://www.drupal.org/project/drupal/issues/3372054#comment-15312495 π updated Claro theme renders badly for some content type edit forms Postponed: needs info . I can also confirm that the patch in #23 resolved the layout issues I described in that issue with text fields of size > 100. Looks like we need a MR for the patch. I can do that momentarily.
We should probably clarify in the issue description that this is relevant on any node add/edit form with a text field that overflows the width in Claro and not specific to sub-theming.
In addition to Chromium and Firefox, I can confirm that the patch resolved it in MacOS Safari.
Yes, thank you @klidifia! The patch in https://www.drupal.org/project/drupal/issues/3381219 π Layout Issues with Claro theme Needs work resolved the issue that I described above and I'm pretty sure is what the original reporter identified. I'm going to mark this as a duplicate to drive others to that issue since it has more followers and is patched, even though I think the title for this issue might be more appropriate.
I'm experiencing this bug as well. If tracked this down to plain text fields with a Textfield size > 100.
Noting that for text fields within collapsible form groups, this bug is perceptible for Textfield size > 87. I am pretty sure that this depends on the browser viewport size though. I am using 1352 Γ 768px in this case.
I'm experiencing this bug as well. If tracked this down to plain text fields with a Textfield size > 100. I was able to clearly reproduce this on a 1920 Γ 980px viewport size by:
- Creating a new content type with only a Title and Body field
- Going to "Manage form display" for the content type and changing the Title field Textfield widget "Textfield size" to "200"
In fact, this can be achieved directly in the browser with developer tools by simply changing the size
attribute directly to "200".
It appears that the size
attribute on the <input>
element somehow overrides the max-width: 100%
for .form-element
causing the grid column width to expand accordingly. The text field itself does not expand.
I've confirmed that this occurs in MacOS Brave, Safari and Firefox, so it is not browser-specific.
In the attached screenshot, I've set the size
attribute to "130" to show the impact on the layout of the secondary region on a viewport size of 1352 Γ 768px.
I have a couple of workarounds, neither of which is satisfactory or generalizable:
- Add this CSS style rule, which sets all text fields to the maximum width of the content region:
.form-element--type-text { width: 100% }
- Configure the Textfield size display to always be 100 or less.
Yes!! Thank you Rajab Natshah! The patch in #5 applied and resolved the error for me on 8.x-1.12 with Drush 12.4.0.0.
I can't confirm the 2.x branch so leaving this status as "Needs review".
The patch in #7 applied to 8.x-2.0 for me. Thank you!
Thank you for linking to that Chromatic post @gaddman! I was able to apply the patch in #2 against the stable 2.0.1 version using
"preferred-install": {
"drupal/insert_view": "source"
since I strongly prefer not to install a dev version on production. Hopefully a maintainer will merge this MR and release a stable Drupal 10 compatible version soon.
Thank you for checking @shiv_yadav. I don't know if it matters, but I'm currently running Drupal 9.5 on PHP 7.4. I updated the description accordingly.
Another reminder to please create a Drupal 10 compatible release. I am on the verge of removing this module from a dozen sites, which I'd prefer not to do.
Restating since I updated the previous comment. #2 works for me and follows Drupal documentation for hook_post_update_NAME
.
amanire β created an issue.
Patch in #4 worked for me. Sure would be nice to get this into an official release.
facets 8001_add_hierarchy_process post-update Add the hierarchy processor to facets that have the hierarchy
I think this is a duplicate of https://www.drupal.org/project/upgrade_status/issues/3345997 π Package x is abandoned Fixed ?
I've just been experimenting with scroll-margin-top
with this module and, unfortunately, it doesn't seem to work as expected when opening to an anchor ID inside a new page. It works fine when that page is already loaded and the anchor link is merely scrolling. I guess that this has to do with the the fact that the IDs are injected into the page with JavaScript after the DOM has already loaded. I would love to be proven wrong, but unfortunately, I think I'll have to find a way to add the IDs server-side to account for this user experience.
@lauriii I assume you mean in in disclaimer.component.yml
? Happy to do that if you can confirm.
This passed testing after enabling the SDC module.
Aha! That's it. So I think we just need to add SDC to core/profiles/demo_umami/demo_umami.info.yml
.
#block-umami-branding is missing? That's very odd, since I had started working on that component first but didn't think I had committed anything and the merge request diff only shows work on the disclaimer. I'll take a closer look when I can find time.
I've just added π Create new SDC component for Umami (disclaimer) Fixed as a child issue.
amanire β created an issue.
Yes! I agree this should at least be configurable. In the meantime, I'll use the patch.