🇺🇸United States @smustgrave

lets try and get this into the next release : )

That is correct, it's just javascript that runs.

On my phone but seems a bunch of eslint formatting was reverted.

Also block commenting shouldn’t be used for inline comments.

Agree sounds like a security risk.

But as a feature request with no follow up going to close out.

Thanks all

Since there's been no follow up and a feature request going to close out. Can always be re-opened

Thanks all

Imagine still valid. Will need to handle when views is not installed though too.

Since there's been no follow up and a feature request going to close out. Can always be re-opened

Thanks!

Added tests

appears to need a rebase.

Going to jump the gun to include in a future release. If not addressed we can re-open.

Lets also fix the pipleine and remove the npm packages we aren't really using.

Fair enough.

Went with a different approach and added test coverage thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

smustgrave → created an issue.

smustgrave → made their first commit to this issue’s fork.

Since there's been no follow up

only question should the current jobs that use yarn, like estlint, be dependent on this new job?

Well that's great stat! Can't argue with the results.

ui_icons provides ui_icons_test which I wonder can be leveraged here.

Left comments on the MR.

Ran the test-only after a rebase https://git.drupalcode.org/issue/drupal-3538681/-/jobs/7977619 and both coverage and update hook are covered.

Applied locally and update hook ran just fine.

Probably a follow up but CSS seems off

But changing the different types worked just fine applying the css class.

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Curious if anyone has used the MR after the last bit of changes?

Not sure I'm a good person to review just bumping priority so fingers crossed 11.4!

Core now uses "symfony/http-foundation": "^7.4" so can we confirm if this is still a bug?

Appears to be 1 thread open and seems valid, should be easy enough and don't think needs to expand test coverage.

Assuming the 11.2 branch will have to be 11.3 now?

Since there's been no response in 2 years to #3 and 6 months to #6 going to close out. If an issue in D11 please re-open.

Thanks all

Appears to be 1 open thread from @dcam @quietone if you want to take a look.

Shouldn't this be broken up some?

Added the remaining tasks section and that the approach needs to be agreed upon.

Appears to need a manual rebase.

Thanks @acbramley!

Since follow up has been created, restoring fixed status. hope that's okay.

Thanks!

It was previously tagged for test coverage which still appears to be needed.

Thanks for reporting as always!

Think next step would be to address the phpstan issue and add test coverage.

Seems straight forward and make sense.

I made the suggestion because the backend and front end dependencies seem different enough I believe would warrant splitting the load.

Know #5 I was referring to a different ticket. I’ve actually seen 2 now where someone identified one of our npm packages had a security release

smustgrave → made their first commit to this issue’s fork.

Thanks, fixed.

LGTM

@berdir thoughts on the latest changes?

Going to go on a limb and say feedback was addressed @nod_ still would like your thoughts though.

Since it's been so long and solution appears to change from years back can we get updated screenshot?

Could this be broken up in actual 2 roles?

One for frontend dependencies, ckeditor stuff, jquery, npm packages
Second for the other stuff like composer and symfony.

Sorry for the delay, can this get a rebase. Gitlab is not doing it so assume there is a small conflict or fuzziness.

Can the follow up mentioned be created

I can confirm this from the screenshot in #16, at first I wasn't able to trigger it but with the user link it was definitely reproducible very easily.

The MR does address the problem in all combinations of user link, shortcuts, and main tab. I did rebase the MR as it was 400+ commits back.

MR is still green

Left some comments on the MR.

Also cleaned up the tags some, most of those aren't in use by core.

Left a comment on the MR around the test.

Pivoting back to this one, should the SVG check be configurable you think?

there a way to revert this or turn off if you don't want this functionality? Get the reasoning to make it appear at the bottom but seems "odd" and kinda looks weird now.

Change to preprocess hook looks good. Guess CR is no longer needed.

Put my old 508 accessibility hat on

Without the patch I get this results

With the patch

It passes AA which I believe is the requirement. Don't believe AAA is on the radar right now.

LGTM.

I see in 11.x we went to 4.0.4 but in 10.6 going to 4.0.5, feel we probably should go to the same version right?

Seems changes were pushed to 10.5 branch, changes should just be going to 11.x

smustgrave → changed the visibility of the branch 3032353-10.5.x-fix-only-backport to hidden.

This broke a number of tests appears, also doesn't have coverage for the change itself.

While I agree about this being on core, it's a small change we can make on this side.

Thanks!

smustgrave → made their first commit to this issue’s fork.

Fine with this change, seems small enough probably don't need test coverage

Thanks!

Definitely would want it to be green. Didn't the icon function come from ui_icons? Should that module be installed.

Update LGTM

Been 2 weeks and don't want to leave this one hanging, so I'll go on a limb.

Seems fine to me.

Seems the tests tag was removed by accident

Re-ran the test-only failures MediaLibraryTest seems like it could be related to this change

Thanks for opening.

MRs need to point to 11.x as the development branch. Kinda seems like a feature request but will leave as a bug, regardless will need test coverage and submaintainer sign off

Since there's been no follow up and a feature request going to close out. Can always be re-opened but didn't seem to have any movement in 10 years, so may be better in contrib.

Thanks all!

Since there's been no follow up and a feature request going to close out. Can always be re-opened

Thanks all.

Think this can be closed as outdated. Most of these routes appear in the pathHooks.php file now under entityTypeAlter()

Since there's been no follow up and a feature request going to close out. Can always be re-opened

Thanks all

Will leave in review for others but think we will still need to know the why part

If this is still desired can the summary be updated with screenshots.

Thank you for sharing your idea for improving Drupal.

We are working to decide if this proposal meets the Criteria for evaluating proposed changes. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or there is no community support. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Wanted to bump 1 more time before closing.

This came up as daily BSI target

Reading the brief summary not sure I'm following what the bug is? If it's not translatable then it not appearing seems correct. Tagging for summary updates

@quietone for 🐛 Twig\Error\SyntaxError: Unclosed """ in "__string_template__xxx" at line 4. in Twig\Lexer->tokenize() (line 228 of vendor/twig/twig/src/Lexer.php Closed: outdated

Since there's been no follow up to #11 going to close out. If still an issue in D11 please re-open

Thanks all!

Thanks but MR needs to point to 11.x as th current development branch.

Also instead of a new test file is there not an existing test that could be expanded?

these kinda issues are tricky in that we need to know the why this key is empty. Else we could be masking a larger problem

Thanks but #3 still is valid

Seems straight forward. Do we still use the needs backport tag?

This seems like it could be a disruptive change based on all the tests that had to be updated to get the MR green.

Also seems like it should be broken up some

LGTM!

Early thoughts?

Leaving the tag for stats

If it’s going to stay summary needs to be updated and will need test coverage as well

Think this is good to go

should include a test.

So this is one I think we actually shouldn't do. I don't want to get into the business or discussions with people about what's considered important.

This came up as the daily BSI target

Think first thing we need is to determine if this is still an issue

If Yes, summary should be updated with the standard template.

Also will need test coverage

Since there's been no follow up, going to close out. If still a bug in D11 please re-open

Thanks all!