🇺🇸United States @smustgrave

If using this module also encourage to check those issues out too. I give credit for meaningful reviews

Waiting to see if other issues get reviewed but will try and deploy this end of the week

Yes added by mistake

So this module is lacking tests but this seems like a change we should have coverage for as this is one of the more important checks

Retested this one again on the performance page and the message definitely helps for me.
Verified the link is correct
Verified the anchor links for the configuration correctly go to the right fields and clicking them moves focus to the configuration.

Think this would be a good improvement

Left some comments on the MR.

@nod_ I tagged you in one if there's a change we want to keep?

Verified threads were captured and active participants were credited.

Looking at the comment // Show the progress bar if the upload takes longer than half a second.

Sounds like it does serve a purpose so unless we can definitely say it's not needed, don't believe we should remove it.

So when I add ComplexDataInterface as the return type in TypedConfigManager.php IDE throws a warning that it needs to be TypedDataInterface

Issue summary still needs cleaning up per #40

Of the points in #49 only 49.2 was one I had a question on. Left the question in the MR but will leave in review for others.

Thanks

Sorry if I missed something but can the issue summary be updated then reviewed?

Seems pretty straight froward.

Re-ran the failing tests and they're passing.

Tweaked the title some.

Rebase seems good.

All MRs are green

Didn't test on every version but on 11.x applied the MR and self-update worked as expected.

griffynh → credited smustgrave → .

Not familiar with other languages but I took that as the letter appears slightly lower then expected/not centered with rest of the word. But just how I took it.

Feedback appears to be addressed here

Left comments on MR.

Curious what's next steps for this one?

What's the request though?

Can the issue summary be updated to include screenshots

Believe this one can be closed out as there has been no follow up to the suggestion #2

So believe views just prints what's in the view and since facets are probably placed by blocks they will have to be manually added.

Should be worth noting that facets 3.0.x is going towards integrating facets with view filters so those should get picked up.

This good to close?

Since it's been 4 years wonder if this is still a valid support request?

Thanks for taking a look @apaderno

Regarding #18 did you run updb? When I applied the MR, before running updb, I saw the error in configuration inspector.

On a standard 11.x profile install
Installed configuration inspector
Applied the MR
Ran drush updb

Reviewing the code thanks @phenaproxima for pointing out scope issue.

Searched for .feedback.yml files and all appear to be updated.

Believe this one is good to go.

This one seems straightforward as it's adding the ignore line.

Hiding patches for clarity.

I'm not seeing the removal of js-hide or js-show in the MR?

Should the issue that’s still in work be removed from 10.3 release notes

Discussed in slack with @catch, for reference https://drupal.slack.com/archives/C04CHUX484T/p1718289507191399

Created a CR just in case.

But does have a legit failure.

How come issue summary was removed?

Thanks for a clear issue summary!

Can we get a change record for the new public function that could be used for others. Sure a contrib module out there maybe could leverage it.

Can verify the related issue users are included in the linked page.

Going to go ahead and mark fixed.

Since this seems to be like something that needs to land in 10.3 or not be needed could we move forward here and open a follow up for more discussion?

Seems straight forward. but can MR be updated for 11.x please.

Coming from 📌 Use String.prototype.includes() instead of String.prototype.indexOf() where necessary Fixed this change already seemed to be accepted so this seems fine.

Left a comment but not sure we can fully remove parameters and add new ones without doing a BC step.

Refactoring suggestions appear to not have broken anything.

Left a comment to confirm that @inheritdoc is inherited, least according to phpstorm.

Updated CR slightly to include additions to the default.settings.php

Looking at the open threads appears @quietone has addressed them all.

Applying MR locally update hook runs fine.

Believe this one is finally ready

Additional items appear to be addressed now. So believe with the combo of the 3 these are complete.

So if I understand correctly this is more about refactoring a temporary solution into a better one, in that case believe task is probably more right category.

Removed the tests tag per #18, if this solution isn't doing anything new then agree probably doesn't need additional coverage.

Thanks for updating the issue summary removing that tag.

Looking at the change and update seems simple enough and didn't break any existing coverage.

Right so I would of expected the test only branch to fail and show the bug

So currently test only feature is passing but should be failing. So not really sure what to do next here.

Backstory, moved away from regions in the header as it really didn't make sense since most themes you can't place whatever you want in the header. So instead turned the header into a large pattern consisting of smaller patterns that can be controlled via the theme.

Good thing still an alpha and can add settings.

Maybe del isn't needed? I used it in the past because recompiling wasn't overriding so ended up with duplicates. Though if this is removed and a scss is deleted it won't be removed from the assets folder.

larowlan → credited smustgrave → .

Del has been a problem package so you aren't alone

Are you running from inside your container?

smustgrave → created an issue.

For me I'm using event_log_track which has a views_data hook.

Very possible I got that setup wrong
https://git.drupalcode.org/project/events_log_track/-/blob/4.0.x/modules...
https://git.drupalcode.org/project/events_log_track/-/blob/4.0.x/event_l...

But whenever I add the relationship to NodeID and try and use the Content type filter it fails and I have to do some hacky workaround of deleting the view and config importing again to recover.

Got bit by this today. Similar to what others have said seems to only be with the Content Type field and when adding a filter. I can add the content type field no issue.

Thanks for taking a look @berdir, if reading correctly there's some feedback about doing before saving.

So tested out on the permissions page and not sure how I feel about it haha. I like the control it gives but also wonder if it should be a theme setting. "Allow users to toggle sticky headers" so if you don't really want it you could turn off.

Functionally though working as advertised.

Leaving in NR for additional feedback

Left some comments

MAY need a change record as this seems like a change that could impact others.

Left a comment but am moving to NW for the title update mentioned in #14

@Grimreaper since we are seeing different things can you post maybe some screenshots of what you're seeing?

Rebase seems fine

Could the issue summary be updated some please. Mainly proposed solution. Assuming not a UI or Data model change.

Also tagging for tests as mentioned in the summary

wasn't notice because there not tests for errors showing up on the batch page

Bummer can't hide 3531 without hiding 8317 so marked in the issue summary

Ran test-only feature https://git.drupalcode.org/issue/drupal-3336994/-/jobs/1841768 which choses all the coverage

Reviewing code and changes make sense.

Only concern I suppose I would have is seeing all the test updates if this would break contrib tests potentially. But doing a deprecation not sure make sense since the link renders a 403.

LGTM

Oh nice so all you have to do is set a key. +1 from me but will let #mglaman mark it since he opened the issue.

smustgrave → changed the visibility of the branch 3336994-stringformatter-access-r2 to active.

smustgrave → changed the visibility of the branch 3336994-stringformatter-access-r2 to hidden.

Can the issue summary be updated to include any proposed solution and other relevant sections from the issue template.

Previously was tagged for tests may be good to get those written and may help guide the fix.

Oh I had read it that maybe it could be included here but cool.

So in regards to this ticket of removing, did a search for .js in css files and found more instances

With the MRs still applied from 📌 Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard Needs work and 📌 Convert use of array() syntax in sentences Needs review ran the grep and got 4 instances
3 believe belong to 📌 Convert use of array() syntax in sentences Needs review so moved that one back to NW
The last one
scripts/dump-database-d7.sh: $result = db_query('SELECT * FROM {'. $table .'}', array(), array('fetch' => PDO::FETCH_ASSOC));

Believe doesn't need to be changed because it's a script?

Going to mark this one.

Actually after applying the MR from 📌 Convert code blocks in comments using array() syntax to @code/@endcode Needs review and ran the grep again think this one missed a few

lib/Drupal/Core/Cache/UseCacheBackendTrait.php:
modules/views_ui/admin.inc:
modules/system/tests/modules/common_test/common_test.module: (Could maybe go in the other one)

List is getting easier to review :)

With the MR still applied from 📌 Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard Needs work

Ran grep -r "\barray(" * | grep -F \* | grep -v "vendor" | grep -v "node_modules"

and got

lib/Drupal/Core/Database/Database.php:   * array(
lib/Drupal/Core/Database/Log.php:   * array(
lib/Drupal/Core/Database/Log.php:   *   $logging_key = array(
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Query/Select.php:   * array(
lib/Drupal/Core/Cache/UseCacheBackendTrait.php:   *   tags. For example, ['node' => array(123), 'user' => array(92)].
lib/Drupal/Core/Datetime/DrupalDateTime.php: * DrupalDateTime::createFromArray( array('year' => 2010, 'month' => 9, 'day' => 28) )
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
modules/views_ui/admin.inc: *   array('dynamic_content', 'section') for this parameter.
modules/system/tests/modules/common_test/common_test.module: * \Drupal::moduleHandler()->alter(array(TYPE1, TYPE2), ...) allows
scripts/dump-database-d7.sh:  $result = db_query('SELECT * FROM {'. $table .'}', array(), array('fetch' => PDO::FETCH_ASSOC));

All these appear to be blocks vs inline so assuming the last one will clean these up.

Left some comments

But tests appear to be passing when assuming they should be failing correct/

Seems straightforward and didn't break anything.

Right but about the BC concern this could immediately start breaking someone's code with no warning.

So applied the MR and ran grep -r "\barray(" * | grep -F \* | grep -v "vendor" | grep -v "node_modules"

lib/Drupal/Core/Database/Database.php:   * array(
lib/Drupal/Core/Database/Log.php:   * array(
lib/Drupal/Core/Database/Log.php:   *   $logging_key = array(
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Query/Select.php:   * array(
lib/Drupal/Core/Cache/UseCacheBackendTrait.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
lib/Drupal/Core/Datetime/DrupalDateTime.php: * DrupalDateTime::createFromArray( array('year' => 2010, 'month' => 9, 'day' => 28) )
lib/Drupal/Core/Entity/EntityAutocompleteMatcherInterface.php:   *   autocomplete API (e.g. array('value' => $value, 'label' => $label)).
lib/Drupal/Core/Render/Element.php:   *   property; e.g., array('#property_name' => 'attribute_name'). If both
lib/Drupal/Core/ImageToolkit/ImageToolkitInterface.php:   *   operation, e.g. array('width' => 50, 'height' => 100,
lib/Drupal/Core/Field/FieldConfigInterface.php:   *   - NULL or array() for no default value.
lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Field/BaseFieldDefinition.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
modules/views_ui/admin.inc: *   array('dynamic_content', 'section') for this parameter.
modules/system/tests/modules/common_test/common_test.module: * \Drupal::moduleHandler()->alter(array(TYPE1, TYPE2), ...) allows
modules/page_cache/src/StackMiddleware/PageCache.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   normally be found in $form_state->getValue(array('wrapper', 'select')),
modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   you would pass array('wrapper', 'select') for this parameter.
modules/views/src/Plugin/views/field/Markup.php: *           array('field' => {$field}) where $field is the field in this table
modules/rest/src/Plugin/ResourceInterface.php:   *   The list of supported methods. Example: array('GET', 'POST', 'PATCH').
scripts/dump-database-d7.sh:  $result = db_query('SELECT * FROM {'. $table .'}', array(), array('fetch' => PDO::FETCH_ASSOC));

Which spot checking appear to be outside of @code blocks.

For #45 follow up is that covered by 📌 Convert code blocks in comments using array() syntax to @code/@endcode Needs review or 📌 Convert use of array() syntax in sentences Needs review ?

Re-ran again and the MR is still passing. Did you mean to push something up?

Issue summary appears to have broken images can those be updated.

Left small comment on MR.

Can confirm on 10.3-rc1, dev branch of Gin with the MR the permissions are now appearing again.

If you go to the views and say use admin theme it should work

Believe it's related to [#3440477]

Confirmed the issue does not appear on 10.3-beta just rc1

smustgrave → created an issue.

Believe this can be closed as outdated.

Today download entity_print via composer and all requirements were downloaded without issue. Entity print appears to be working as advertised

For the maintainers, with D9 unsupported down this may be outdated

Based on another ticket where a user reported issue with persian characters could this be a bug with dompdf?

Wonder if this can closed or marked fix? Seems request has been answered.

Could more steps be added by chance? But thanks to @Matt B in #3 seems like this might be an issue with dompdf maybe an issue should be logged there instead?

This seems like a simple change and can confirm the issue using configuration_inspector. The whole schema could be validated but this is the only error in entity_print.print_engine.dompdf

Tested MR following the steps and revision is being printed perfectly.

Something probably that should have simple test coverage.

Don't see any file for moderation so probably could add one to Functional or Kernel folders.

If the maintainers want to keep this type of issue then probably should be rescoped to cover the warnings from gitlab.

Personal recommendation would be to close this out and start a new one since these type of tickets come off as farming.

Had some time so went ahead and applied the change mentioned in the MR.

Believe all points have been addressed. Left open for additional reviews for about a month so going to mark.

Did retest and still behaving as advertised.

Seems to just be a rebase so will restore.

Sounds like a good plan, updated issue summary

Thanks for reporting

Moving to 11.x as the current development branch, MR will have to be updated as well.

Issue summary should follow standard issue template

There are a test scenario that can be added to show the issue and make sure it doesn't come back.

Thanks.

Sounds like the states api needs some work for that scenario

Just make sure to read the release notes. Really only difference is jquery modules are removed so if you’re using them you’ll have to include in your composer file.

Seems linkit started a new branch for D10/D11 so should this start a 3.0.x branch that drops D9 support and < php8.1

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