Account created on 17 August 2022, almost 2 years ago
  • Associate Engineer at AcquiaΒ 
#

Merge Requests

More

Recent comments

The if condition that i added is just opposite of what's mentioned in #31 but still it work.I will try to deep dive more into that.

Rebased and also marking it needs review as all the feedbacks seems to be addressed.

Addressed all the feedbacks in #145.
Regarding

and some unexpected changes in core/themes/claro/css/theme/media-library.css

the changes are made to make the pencil visible in high contrast setting of the browser.Before that it was styled using inline css.
For now I have removed the use of stroke and used fill.

Attaching the screenshots of the state when the stroke/fill is not used

Utkarsh_33 β†’ changed the visibility of the branch 3422758-focus-latest-dialog to hidden.

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

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

Here are the steps that i followed to reproduce the issue.
1)Visit node/add/article.
2)Click on the any of the vertical tabs for ex:- Menu settings.
3) Try scrolling on the page.

After following the above steps i didn't experience any lag in scrolling.

#28 seems to be a better approach, so I have reverted the class changes and added tabIndex to the button thus making it non tabbable.Also the mousedown trigger event is working so marking it as needs review again.Thanks!

I tried reproducing this but i was not able to do that on 11.x .

Utkarsh_33 β†’ changed the visibility of the branch 3447611-claro-theme-loses-test-only to active.

Utkarsh_33 β†’ changed the visibility of the branch 3447611-claro-theme-loses-test-only to hidden.

Can we use regex and string function like replace to get it more consitent with the PHP's trim function.

console.log("\u00A0".replace('/^[\t\n\v\f\r ]+|[\t\n\v\f\r ]+$/g', '').length)

give the results comparable with https://3v4l.org/HHjqS.

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

@smustgrave if you look at the tests,it asserts that the button is not present.As Ben mention in #9 πŸ› Claro theme loses actions for the view bulk operations form Needs review that we only want a test to confirm that the issue exist but not the fix(alteast latest till #9).

Also

core/modules/system/tests/src/FunctionalJavascript/Batch/ProcessingTest.php

test was added in https://www.drupal.org/project/drupal/issues/3406612 πŸ› Exceptions in batch no longer are shown on the page: Javascript error Fixed issue with a temporary fix which is now removed and replaced with a concrete JS fix.
If Still there is a requirement for adding test I am happy to do that.

No I didn't push anything. In #14 i just explained the reason for using the current state of MR.

Added a test module that alters the form and also added test that verifies that the issue exists and also covering the solution.

I initially tested that with the same logic in this commit but it was failing on the gitlab(here) but passing locally.
But with the suggestion provided in #13 it passes locally.

I debugged this for a while and here are some points that i observed:-
1)$generated_link = $link_generator->generate($element['#title'], $element['#url']->setOptions($options)); in
Link.php is called twice for the link.For the first time it get the wrong element without the data drupal selectors but it does not attach that to the markup(which is on the next line).For the second time when this is called again $element['url']->setOption('set_active_class', TRUE); sets the class to true which inturn sets the correct data attributes to the element and then attaches it to the mark-up in $element['#markup'] = $generated_link; in Link.php.
2)With the correct element set at the PHP side It still does not correctly render the element in the markup.

Just wanted to confirm if I am on the right track of figuring out what could be the issue?

Created a new branch pointing to 11.x branch and also added the test for the fix.

I'm sorry i messed this up because of the target branch.Will fix this soon

The test are fixed now.In response to

Let's drop the changes to filter.filter_html.admin.js: The whole thing is deprecated and will be removed in D11. #2567801: Deprecate core/modules/editor/js/editor.admin.js JS APIs in Drupal 10, for removal in Drupal 11

06c9630b

commit removes the whole file but as far as i understood from the comment is that we just wanted to revert the changes from that file.It would be helpful if @nod_ can verify the change.
Marking it needs review as rest everything is addressed apart from the changes in 06c9630b commit.

Added z-index according to

Update the Z-index of the dropdown, but target something specific to autocomplete vs. the current approach of targeting ui-frontIf an autocomplete dropdown is open, it's reasonably safe to assume it should be above other elements. An autocomplete-specific z-index boost (vs ui-front) is easier to test and less at risk of causing unwanted side effects

.

I reverted the change made in #3350972: [random test failure] Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections() πŸ› [random test failure] Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections() Fixed so that we are back to the original use of debounce functionality as discussed in #17 πŸ› Layout builder off-canvas positioning problem when resizing browser Needs work and the test are also passing.

I have added a custom class to take of all the things described in the remaining tasks.I was able to hit the breakpoint for

Cross-browser manual testing - Confirm the trigger('mousedown') in Drupal.fieldUIOverview.AJAXRefreshRows() still works, in all our supported browsers.

atleast in Chrome.
The failing test seems to be random failures as they are failing on 11.x as well(atleast on my local).Marking it as needs review just to get an initial round of review.

The current MR works only if we change table.sticky-enabled to table.position-sticky in this

function tableHeaderInitHandler(e) {
    once('tableheader', $(e.data.context).find('table.sticky-enabled')).forEach(
      (table) => {
        TableHeader.tables.push(new TableHeader(table));
      },
    );
    forTables('onScroll');
  }

.
After this is changed then the recalculate function is invoked as expected.The problem with this is that the table is still overflowing out of the details element as you can see in the SS.

Production build 0.69.0 2024