๐Ÿ‡ฎ๐Ÿ‡ณIndia @srishtiiee

Account created on 13 August 2021, over 2 years ago
  • Associate Engineer at Acquiaย 
#

Merge Requests

More

Recent comments

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

@phenaproxima, thanks! Changed the implementation to injecting a logger instead. Also, added console logging for each config action and config import. It needs another review.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Added basic logging to the recipe application workflow. The MR needs an initial review for what more can be done here to enhance the debugging process.
Standard recipe application logging looks like this RN:

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

@phenaproxima, thanks for the review! I've addressed it and the MR needs another review.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Addressed the feedback, setting back to NR

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Addressed feedback, setting back to NR.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

I intended to move the whole caching layer one level down to the plugins instead of having a common layer for all the source plugins. I might have incorrectly interpreted what @tim.plunkett had suggested but the idea was to have the plugins handle their caching individually/separately which is what I tried to do by moving it into the MockDrupalDotOrg plugin.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

That's a random test fail, marking NR anyways.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

The second step in case of ungrouped field types mentioned in #3 isn't implemented yet. It will be affected by the issue that removes the radio buttons from the field cards in the first step: ๐Ÿ“Œ Field selection breaks conventions and increases cognitive load Needs review . I think we should wait on those changes to be committed to avoid re-work here.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Back to NW for a minor change. The MR looks neat otherwise after the multiple confusing keys have been cleaned up.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Updated the issue summary to remove all the stuff that has been taken care of in other issues.
Removed the Upgrade path tag since the admin UI views are owned by the site, and we don't need to provide for it. If at all the upgrade paths for the field is required, it can be done in a follow-up.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Updated the issue summary and created a change record.
Also, I followed the same steps mentioned in #53 and the page renders as 2 columns with the MR applied as well. The modifications to the configuration are saved and rendered precisely on the nodes too.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Setting this back to NR as I manually tested the MR and the focus is indeed on the Current password field after clicking Save. @smustgrave Do you have any additional details/steps to reproduce the issue?

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Updated the issue summary to restrict the scope of this issue to have only the default region mapping and move the mapper UI to a separate issue [See #48 โœจ Allow changing the layout of an existing section in Layout Builder UI Needs work ].
Removed the Needs tests tag as the mapper UI testing isn't required anymore.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Moved the 'enabled' checkbox to main area, changed label for the details element that only has the 'expanded' checkbox and added support for 'description' in edit form along with the add form.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

@benji thanks for pointing that out that css file (node.module.css) is never used, created a follow-up to remove all references to it ๐Ÿ“Œ node.module.css is obsolete and can be removed Active . I removed the template_preprocess_hook from this MR. Also, the label for the details element containing the checkboxes is changed to "Display settings" as per the suggestion.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Addressed the feedback.
Also fixed the border to match node edit form.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Addressed the feedback and update issue summary.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Reviewed the newly added BC layer for the field storage form alter hook and the changes look good. Moving back to RTBC.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Manually tested all the core field types again, and everything works as before.
- The #states related bug in Files displayed by default field in the FileItem storage settings is now fixed by 0c3d584e.
- Throughly tested the more complex field types such as entity reference, selection lists, file and image and those seem to work fine as well.
- The image field type has a Default image field appearing twice now that the forms are combined (one from each: field and field storage forms) which IMO unnecessarily makes the form lengthy. However, it is out of this issue's scope and can be removed from one of the forms in a follow-up.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Addressed all the feedback.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

The weight element in the menu link edit form needed to be wrapped in a container to be grouped along with the other secondary elements.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Looks good! Left a minor feedback. Also, this most probably requires a change record.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Added an upgrade path and a test for the new formatter setting.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

While the 'weight' element is grouped as a secondary item, it unexpectedly appears in the primary section of the menu link edit form, rather than being grouped with the other secondary elements in the column. I'm unsure about the reason behind this behavior and would appreciate some insights.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

srishtiiee โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Added a failing test demonstrating the issue with the default values widget in case of list type fields.

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

Added the isNew check to FieldStorageConfig::hasData() and created a CR for it.

Production build https://api.contrib.social 0.62.1