May be over engineering but should it be a configuration option for something in the settings.php that could be set?
The related issue is scary lol but believe this one is good to go. Much cleaner too :)
Wanted to bump 1 more time if still valid? Still a major?
since there's been no follow up and this kinda felt like a feature request vs a task going to close out. If I'm wrong please re-open.
Method still appears to return object or false so probably still valid.
since there's been no follow up and as a feature request going to close out. Can always be re-opened
Also know we are phasing out jquery best we can.
Since there's been no follow up going to close out. If still an issue in D11 please re-open!
@acbramley for 🐛 Node access record filtering in views queries Active
Since there's been no follow up for steps going to close out. If still an issue in D11 please re-open updating summary with steps.
Thanks all!
@kovalevm did you check your theme as mentioned in #4?
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 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 reporting this problem. We rely on issue reports like this one to resolve bugs and improve Drupal core.
Since there has been no activity here for over 8 years we are asking if this problem persists on a currently supported version of Drupal. To help, add a comment explaining if the problem still occurs or not. Any extra detail you can provide can help others who experienced this.
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!
Was the original goal to give a token example? Is there a different token that could be used?
Possible to get a small test covering this change?
MR appears to have pipeline issues
With regards to the post_update hook this something may want to use a ConfigImporter class to run in batches?
Post update hook will need test coverage too
Thanks!
So I'm sure these fluctuate but compared this run to the last run from HEAD
Javascript #1 3 min 27 sec compared to 3 min 41 sec
Javascript #2 3 min 7 sec compared to 5 min 12 sec
Javascript #3 3 min 34 sec compared to 2 min 53 sec
So definitely improvement in 2/3 of the runs. The #3 may have been an outlier since there's such a difference in #2
Going to mark as a net improvement.
So did some manual testing.
On a fresh 11.x install
Standard profile install
Created a few Articles and Basic page content
Ran php core/scripts/drupal content:export node --dir=my-content --bundle=page
Verified ONLY Basic pages were exported
Did the same for Articles
Appears to be working as expected!
Tagging for a CR though.
Leaving in review for others
re-reading #36 also mentions a release note
If no one else has a concern #39 it can be ignored.
I found the issue this was done in ✨ Use modal in add new field flow Active as I was surprised this approach of a try/catch was accepted but that was a pretty significant change so maybe just lost in the shuffle.
Seems like a good refactor
I did have to look up where 'General' came from but that was easy.
Now that we have the Development settings page that isn't stored in config we could extend that form to set
$settings['extension_discovery_scan_tests'] too ?
Believe the merge conflict is valid now in core/.deprecation-ignore.txt. Bot occasionally has it's moments.
Ran the test-only run here https://git.drupalcode.org/issue/drupal-3532360/-/jobs/6995154 to show the coverage
Left a comment on the MR.
If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
With or without the MR I'm getting
Something maybe changed on the ckeditor5 side
smustgrave → made their first commit to this issue’s fork.
Still seems relevant
Tagging for an IS update if the idea to deprecate 1.
Since there's not been a follow up and as a feature request going to close out. Can always be re-opened
Thanks all!
Think need to re-look at this. All the test changes seem like this would be an immediate breaking change for contrib and custom tests.
Example
$page->fillField('label', 'Bears again');
Is very common so we should still be able to handle this
Think we should be logging an error if the role doesn't exist.
Resolved what threads I could. But don't want to bypass @xjm review. xjm do you mind checking your open threads please?
Not to be used by core but uploading a txt file of the mjs file I used on a contrib theme using eslint-config-airbnb-extended
ChatGPT did help some.
All I did was rebase but appears some upstream changes broke the unit tests. Left a small comment that should address them.
I meant LoggerChannelTrait;
. Then just need to call $this->getLogger()->warning()
smustgrave → made their first commit to this issue’s fork.
CR reads fine to me. Can't wait to add this to some of my ui_suite_uswds components. I didn't mark fixed as didn't want to jump the gun.
Left a small comment but leaving in review for others.
This was provided to me in slack https://webaim.org/techniques/keyboard/
Ensure elements with ARIA role="button" can be activated with both key commands.
I can confirm that tabbing through the toolbar space now opens the manage section.
Will say from a user standpoint the previous behavior of using space scrolled the page down and I thought that was better. But acknowledge the accessibility rule.
Moving to NW for the change record.
smustgrave → made their first commit to this issue’s fork.
IS still appears incomplete needs to use the issue template.
Not reviewed.
Revert the previous test but ran the test-only job https://git.drupalcode.org/issue/drupal-3553853/-/jobs/6994061
So I actually got the wrong test but took the feedback from that one to the correct one.
Original concern of updating the the twig templates directly is back. We will need a CR as contrib and custom themes won't have that change.
In that case I would be a +1 but would be more comfortable is maybe @catch could give it a look. I'm happy to mark though so he can still merge.
Maybe we could add a simple warning or message if content for the targeted node already exists?
This came up as a daily BSI target.
This seems like a task vs a bug but agree maybe something can be done?
Wanted to bump 1 more time for steps else we could close.
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!
Was going to tag stale-issue-content but this seems like it's probably still relevant.
Thank you for reporting this problem. We rely on issue reports like this one to resolve bugs and improve Drupal core.
Since there has been no activity here for over 8 years we are asking if this problem persists on a currently supported version of Drupal. To help, add a comment explaining if the problem still occurs or not. Any extra detail you can provide can help others who experienced this.
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!
I could for sure! I get a few days a month from work for contribution work.
As mentioned probably would close out the D7 stuff because that's done.
For the D10+ stuff I see you have amazing test coverage so would continue on that path for all fixes or new features.
Cleaned up the IS some but also dove into the history and that was quit a hole lol. Agree I can't find a reason why it was cutoff. Seems was added as part of a security release and since @prudloff is on the security team and doesn't see a reason to cutoff think this fix is fine.
Outputting in the steps
Nightwatch
cSpell
Eslint
Stylelint
Makes sense to me.
Verified it works
Just going to post questions
1. I'm seeing the deprecations are changing in WorkspaceManager but believe these changes have already been in a release so is that okay?
2. I see we are switching to cron and limiting to 50, which makes sense. My question is if those workspaces are large with lots of content each any concern using cron then?
3. Do we need a CR for the new Event and Subscriber?
Manual tests I did
Base setup.
1. Latest 11.x
2. Standard profile install
3. Apply MR
4. cleared cache
Managing spaces
I created a new workspace (dev)
Switched to it
Switched to stage
Deleted dev
Working
Publishing content
Switched to stage
Created an Article (Test stage)
Switched to live
Made sure I could not edit Test stage
Switched to stage
Published content
Seeing the Test stage page in live space
Working
Functionality wise appears to be working.
I don't want to derail this at all. Is there functionality from linkit that won't be covered by this change? Just to help evaluate using this later.
Believe what @catch was referring to was this needs a post_update hook. The schema entry nullable is saying the value can be null (which may not be correct) but the key is still missing from existing views. Here's a scenario
Do a config export of a view
Apply this MR
Save the view again making 0 changes
Export the view again and notice it has changed because this key was added but I didn't do anything.
So this needs a post_update that uses a configImporter to help. This way it's ran in batches because some sites could have 100s of views. An example would be views_post_update_table_css_class which calls a function in the viewConfigImporter
Then the test coverage for that would be something maybe like testConfigUpdate().
1. Check view doesn't have this new key
2. Run updates
3. Check view does have this new key
Hope this helps.
Before
After
I also checked Require email verification when a visitor creates an account which should have aria-described
Ran the test-only job here https://git.drupalcode.org/issue/drupal-2839344/-/jobs/6981702 which shows the coverage
Believe this one is good to go.
Since there's been no follow up to how this is an accessibility issue I'm going to close out. If still valid please re-open and update summary with how this is an accessibility issue please
Thanks all!
Since there's been no follow up and as mentioned there is a contrib module. Slimmer core stronger core :)
Actually don't want to ping pong this I'll make a new ticket.
Responded but that change is needed to fix a ckeditor multi line test. Seems like an oversight as the text with summary placeholder was type text
Hoping to do a release end of the week so if we can get test coverage we can include this.
Just wanted to see if you've had any luck (if using on a project) @macsim
Lets postpone till a fix is found for 🐛 Datepicker doesn't register relative dates Active
Will include in 7.1.x as does seem like a net gain improvement.