πŸ‡ΊπŸ‡ΈUnited States @ricksta

Account created on 7 June 2008, over 16 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States ricksta

I've updated the changes to work with the 2.x branch. Tests running.

πŸ‡ΊπŸ‡ΈUnited States ricksta

ricksta β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States ricksta

Below is a new report of the results of the Drupal 11 check for:

  1. Check for Drupal 11 code compatibility
  2. Check for PHP 8.3 compatibility
  3. Check for jQuery 4 compatibility
# Drupal 11 compatibility audit: breadcrumbs_visibility


## References
- Drupal 10 deprecations: https://dev.acquia.com/drupal10/deprecation_status/errors
- jQuery 4 deprecations: https://blog.jquery.com/2024/02/06/jquery-4-0-0-beta/

## Composer
- [x] This module does not have a composer.json file

## PHPStan Audit of Drupal deprecations
```

 [OK] No errors                                                                                                         

```

## Drupal-Rector Audit of Drupal deprecations
```

 [OK] Rector is done!                                                                                                   

```

## Audit of deprecated PHP <8.3 calls
If no errors are listed below, php-compatibility did not find any.
```



Time: 942ms; Memory: 8MB

```
## jQuery 4 compatibility: List of all custom JavaScript files
Any files listed below should be reviewed for [jQuery 4 compatibility](https://blog.jquery.com/2024/02/06/jquery-4-0-0-beta/)
```
```


## Diff for updating core_version_requirement
```diff
```

πŸ‡ΊπŸ‡ΈUnited States ricksta

Hi @sarwan_verma, I just wanted to note that as written above:
"This module uses Gitlab CI for automated testing of proposed code changes. To contribute code changes, create a merge request. Patch files will not trigger automated testing and are not sufficient for review."

πŸ‡ΊπŸ‡ΈUnited States ricksta

ricksta β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States ricksta

"Let's brainstorm where our default themes could be improved..."

Could we be specific and mention what themes we mean? Olivero, I see is marked obsolete. Back-end, front-end themes? Are any themes a priority to look at first?

πŸ‡ΊπŸ‡ΈUnited States ricksta

Testing the change in the merge request, I find that I can't see an embedded Soundcloud file, either in the CKEditor window or on the page. The wrapping element around the <iframe> needs to have some value for "padding-bottom," but instead has a NULL value, so the display appears invisible.

πŸ‡ΊπŸ‡ΈUnited States ricksta

ricksta β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States ricksta

The patch works fine for 6.1.x.

πŸ‡ΊπŸ‡ΈUnited States ricksta

This patch contains the new files. I've tested with url_embed 2.0.0-alpha2 release.

πŸ‡ΊπŸ‡ΈUnited States ricksta

Sorry, that last patch won't do anything for making this module work responsively. But this patch should. I've tested with url_embed 2.0.0-alpha2 release.

πŸ‡ΊπŸ‡ΈUnited States ricksta

This patch should work against the 2.x-dev version. I've tested with url_embed 2.0.0-alpha2 release.

πŸ‡ΊπŸ‡ΈUnited States ricksta

This is fixed but a new problem is noted: Button with id|name|label|value "Save content type" not found. https://git.drupalcode.org/project/breadcrumbs_visibility/-/jobs/582166

I'll create a new issue to update these tests.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I'm not experiencing any problems with Apple VoiceOver. Skip link is being read.

MacBook Pro Sonoma 14.1
Drupal version 11.0-dev

Private windows opened in:
Google Chrome: Version 119.0.6045.199
Firefox: 120.0.1 (for this I used Chris Pederick's Web Developer tools simulator for iOS 12/13, Pro max IOS 114.6)

πŸ‡ΊπŸ‡ΈUnited States ricksta

Could I get please get more information on this, maybe someone layout the steps reviewers should follow to see the bug? I'm not seeing a problem. I see patches for the Seven theme which is marked as deprecated and not installed on a fresh D9.5 site.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I have nothing new to add here other than to say I support the approach and points brought up in 300 πŸ› "Unpublished" style of rendered entities is not accessible (and looks bad) Needs work .

πŸ‡ΊπŸ‡ΈUnited States ricksta

I'd like to be able to test the patch. But I've followed the "Steps to reproduce" and I can't reproduce the bug.

πŸ‡ΊπŸ‡ΈUnited States ricksta

Updating documentation for how to change the breakpoints.

πŸ‡ΊπŸ‡ΈUnited States ricksta

Addressing the #3 comment, here's a patch for the README.md file with instructions for changing the CSS breakpoints. After this is RTBC and merged, I believe we can close this issue.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I strongly agree with the approach in #16 above. The other alignments mostly utilize adding a CSS class, too. Though I notice a data-align="right" in the CKEditor "source" becomes class="align-right" when a page is saved. So, I'm not sure what that attribute would be that would then become a CSS display: inline. I also agree with comment #17 that the default should be one where the adjacent content doesn't fall to the bottom.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I have reviewed the patch in #12 πŸ› No support for non-Latin characters (such as Cyrillic) Fixed in both Drupal 9 and 10 and can confirm it works fine.

πŸ‡ΊπŸ‡ΈUnited States ricksta

All changes are now in 2.x branch, so this is closed as fixed.

πŸ‡ΊπŸ‡ΈUnited States ricksta

Updating the title to more accurately reflect the issue.

πŸ‡ΊπŸ‡ΈUnited States ricksta

My organization has been using the latest patch on multiple sites in production, without incident, for over a week, so I think this patch is good to go.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I see another issue for remaking the README.md file to suit template standards, so I feel this can wait until that issue is completed.

But here's an idea of what I'd like to see:

Using Linkit on a link field
To use Linkit on a custom link field, you must configure the widget on the "Manage form display" page by selecting the "Linkit" option in the dropdown.

πŸ‡ΊπŸ‡ΈUnited States ricksta

I believe you missed one of the maintainers : Mark Fullmer (mark_fullmer). Can you include him?

πŸ‡ΊπŸ‡ΈUnited States ricksta

I can confirm the patch works. Screenshots attached.

πŸ‡ΊπŸ‡ΈUnited States ricksta

This fixes the problem caused by the patch in https://www.drupal.org/project/linkit/issues/3236385 πŸ› Form button obscured if Layout Builder is being used Fixed without a regression. I've checked the Linkit works in the WYSIWYG editor and in a link field.

πŸ‡ΊπŸ‡ΈUnited States ricksta

The following were done:

  • Text manually word-wrapped within around 80 cols
  • Added line breaks
  • Added rest of maintainers
πŸ‡ΊπŸ‡ΈUnited States ricksta

This looks great overall. I'd suggest one thing, and that is to put a <br/> after both the "Q:" in the FAQ section for better readability.

Here's what you've got now:

And here's my suggestion:

πŸ‡ΊπŸ‡ΈUnited States ricksta

Comment #50 is correct that this commit introduced a regression in the CKEditor interface. I've removed the margin and text-align code as per Wim Leers suggestion and it does fix the issue he pointed out. First screenshot shows after the last commit was made.

This screenshot shows the results of the code change to .ck .drupal-media demonstrating the image is left-aligned and visually appears left-aligned.

The screenshot below shows that the text-aligning to center is still working and there are no regressions.

I should note that there are margins left and right of 40px that make it look like when the image is left-aligned or right-aligned, that the image is not flush with the editing window. These margins come from the user-agent style sheet. Should we override this behavior?

Production build 0.71.5 2024