Account created on 29 January 2020, over 5 years ago
  • Engineer - Frontend at QED42Ā 
#

Merge Requests

More

Recent comments

šŸ‡®šŸ‡³India meghasharma

I was able to reproduce this issue as described.
Steps followed:

- Created a code component named "myCode".
- Added it to the component list.
- Deleted the "myCode" component from the list.
- Tried to create a new code component with the same name "myCode".

The error message appears: "Cannot read properties of undefined (reading 'startsWith')" and the creation fails.

This seems like a bug in how deleted component names are handled in memory or state.

šŸ‡®šŸ‡³India meghasharma

Additional observation while reproducing the issue:
After making any changes to the image field via the Page Data panel — for example:
Uploading a new image
Removing the image

...the "Review X changes" button at the top still shows ā€œNo changesā€.
Ideally, these actions should trigger the button to update and show "Review 1 change"

šŸ‡®šŸ‡³India meghasharma

I was able to reproduce this issue.
When editing an Article content's Experience Builder Page Data panel:

- If the image is not changed, the Alternative Text field works as expected and is editable.
- But when I remove the existing image and upload a new one, the Alternative Text field becomes non-editable — it shows up, but I cannot enter or type anything in it. No characters appear.

šŸ‡®šŸ‡³India meghasharma

Thanks @penyaskito for the clarification!
Yes, the client-side validation is now working correctly and shows an error when the alias does not start with /.
Since the publishing behavior is related to #3521759 and not this issue, I’m moving this back to Needs review.

šŸ‡®šŸ‡³India meghasharma

Updated the patch to add client-side validation to ensure alias starts with /.
However, currently we’re still able to publish even if the alias doesn’t start with /
Attaching screenshot to show the client-side error.

šŸ‡®šŸ‡³India meghasharma

I was able to reproduce the issue on my local setup. When switching between pages using the navigator, the canvas preview incorrectly displays "Untitled page", even though the correct title appears in the right-hand panel.

After applying the merge request, the canvas preview now correctly shows the updated page title when switching between pages.

Attaching screenshots for both before and after the fix for reference.

šŸ‡®šŸ‡³India meghasharma

Issue was from my end. Ran composer install properly and issue is resolved.
Thanks.

šŸ‡®šŸ‡³India meghasharma

Reopening this issue because issue added a hard-requirement on JsonSchema/Validator.
Getting error while clearing cache.

šŸ‡®šŸ‡³India meghasharma

Right now I’ve added backend validation to ensure alias starts with /, which resolves the layout error.
Also noticed that the error message ā€œThe alias path has to start with a slashā€ is already being shown in the UI if / is missing — but the page still gets saved.
Should I go ahead and update the frontend logic to block submission if the alias is invalid?
Attaching the screenshot.

šŸ‡®šŸ‡³India meghasharma

Thanks @akhil for the input!
You're right — adding frontend validation makes sense for better UX and to prevent invalid input early on. I'll update the PR to include this validation inside validateNewValue().

Also, I think we can keep the backend fallback (str_starts_with) check just to be safe in case any invalid alias somehow bypasses the UI (e.g., via API or future form changes).

Let me know if you'd prefer removing the backend fallback.

šŸ‡®šŸ‡³India meghasharma

I have verified the changes related to renaming SdcPropToFieldTypePropMatcher to JsonSchemaFieldInstanceMatcher and SdcPropJsonSchemaType to JsonSchemaType.
1) Old class names (SdcPropToFieldTypePropMatcher, SdcPropJsonSchemaType) have been completely removed from the codebase.
2) New class names (JsonSchemaFieldInstanceMatcher, JsonSchemaType) are being used consistently throughout the module.
Everything looks good

šŸ‡®šŸ‡³India meghasharma

Added comments explaining why the properties are protected to support DependencySerializationTrait.

šŸ‡®šŸ‡³India meghasharma

#22 Yes, because Install and set Claro as the admin theme in XbTestSetup changes already merged in 0.x main branch. may it's done in other issue.

šŸ‡®šŸ‡³India meghasharma

Rebased the branch, yes there are changes already merged in 0.x
only needed to do e2e tests we no longer need to manually enable claro, have done in MR
Please review

šŸ‡®šŸ‡³India meghasharma

Rebased the branch and setting the admin theme to Claro came as incoming changes.
please check

šŸ‡®šŸ‡³India meghasharma

meghasharma → changed the visibility of the branch 3514551-consider-enabling-claro to active.

šŸ‡®šŸ‡³India meghasharma

meghasharma → changed the visibility of the branch 3514551-consider-enabling-claro to hidden.

šŸ‡®šŸ‡³India meghasharma

meghasharma → changed the visibility of the branch 0.x to hidden.

šŸ‡®šŸ‡³India meghasharma

meghasharma → changed the visibility of the branch 3514551-consider-enabling-claro to hidden.

šŸ‡®šŸ‡³India meghasharma

I tested this issue on my local setup by following the steps mentioned in the issue description. After duplicating the "Test Article" page using the navigator, the new page titled "Test Article (Copy)" was created successfully. In the right-hand editor panel, the Title field was correctly populated with "Test Article (Copy)" and did not appear blank.
Attaching a screenshot for reference.

šŸ‡®šŸ‡³India meghasharma

I read the issue and noticed inconsistent component names. It would be better to standardize the naming convention. Once we decide on the format, I'll update the names.

šŸ‡®šŸ‡³India meghasharma

@larowlan, I think we should move the pageDataExists check inside as it's more specific to page data forms.
I will update the MR with this change. Please confirm if this approach looks good.

šŸ‡®šŸ‡³India meghasharma

Updated the error messages in ErrorCodesEnum::getMessage() as per feedback from @tedbow.

šŸ‡®šŸ‡³India meghasharma

I've updated the messages in \Drupal\experience_builder\Controller\ErrorCodesEnum::getMessage() to make them more helpful and actionable.

šŸ‡®šŸ‡³India meghasharma

The PHPStan error regarding the $prop_source parameter still needs to be fixed.

šŸ‡®šŸ‡³India meghasharma

Updated the regex to apply case-insensitivity only to the file extension part using (?i:...) instead of applying it globally.

šŸ‡®šŸ‡³India meghasharma

Updated the regex for image-uri and stream-wrapper-image-uri to be case-insensitive using the (?i) flag.
This allows image extensions like .JPG, .PNG, etc., from Media Library to work correctly and fixes the Twig rendering error.
Please review, Let me know if there’s a better approach we should consider.
Thanks @dhruv.mittal for the helpful suggestion!

šŸ‡®šŸ‡³India meghasharma

Converted all ConfigEntityType plugins to use PHP 8 attributes instead of annotations.

šŸ‡®šŸ‡³India meghasharma

Renamed `PropSourceComponent.field_data` to `.propSources`.

šŸ‡®šŸ‡³India meghasharma

Renamed state, prop and component data attributes.

šŸ‡®šŸ‡³India meghasharma

Added a "Components" tab under the Appearance section and updated the "Enabled Components" and "Disabled Components" tabs under the "Components" section by adding the parent_id., Moved "Page builder components" listing in Structure to "Components" in Appearance.

šŸ‡®šŸ‡³India meghasharma

Changes Done:
Updated Permission: experience_builder.component.status now uses the administer themes permission.
Updated Links: Changed /admin/structure/component to /admin/appearance/component in component entity definition links.
Removed "Page Builder Components" from Structure.
Please review

Now:
You can see the list of enabled and disabled components at /admin/appearance/component.

Remaining Task:
Add a "Components" tab under the Appearance section.

šŸ‡®šŸ‡³India meghasharma

Could you please add some more details to the issue description? It would be helpful to have more clarity on the expected behavior and a proposed solution.

šŸ‡®šŸ‡³India meghasharma

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

šŸ‡®šŸ‡³India meghasharma

Fixed the issue where long component names were overflowing the badge. Now, the text stays within bounds and displays correctly. Ready for review.

šŸ‡®šŸ‡³India meghasharma

This is already implemented in the file. The datetime field’s date and the file_uri field’s url are handled the same way as daterange in tests/src/Kernel/EcosystemSupport/FieldTypeSupportTest.php.

'datetime' => [  
  // šŸ› Core bug: this is the computed equivalent of `value`, should be marked internal.  
  // @todo Give this similar treatment as `daterange` in https://www.drupal.org/project/experience_builder/issues/3512853  
  'date' => FALSE,  
],  
'file_uri' => [  
  // šŸ› Core bug: this is the computed equivalent of `value`, should be marked internal.  
  // @todo Give this similar treatment as `daterange` in https://www.drupal.org/project/experience_builder/issues/3512853  
  'url' => FALSE,  
],  

Let me know if anything else is needed. Thanks!

šŸ‡®šŸ‡³India meghasharma

Marked 'date' (DateTimeItem) and 'url' (FileUriItem) properties as internal
Created DateTimeItemOverride.php to override DateTimeItem and marked the date property as internal.
Updated FileUriItemOverride.php (already present) to mark the url property as internal.
Please review

šŸ‡®šŸ‡³India meghasharma

Tried conditionally rendering the div using {initialized &&

...

} instead of just applying display: none, but the issue persists. The console still shows the 'Infinity' warning for height. Keeping the original approach with isFinite checks seems to work.

šŸ‡®šŸ‡³India meghasharma

Instead of directly manipulating the DOM using document.querySelector and menu.style.display = 'none' (lines 84 and 88), we should use Radix UI’s built-in method (if available) to close the menu.

Can we avoid direct DOM manipulation? Radix UI might change internal class names, which can break the code.
Can we use state for menu visibility? The menu’s visibility should be controlled using useState.

const [open, setOpen] = useState(true); // Control menu state

const closeContextMenu = useCallback(() => {
  setOpen(false); // Close the menu using state
}, []);
šŸ‡®šŸ‡³India meghasharma

I have updated @typescript-eslint/eslint-plugin and @typescript-eslint/parser to 8.26.0 as per the issue requirement.
Installed versions:
@typescript-eslint/eslint-plugin: 8.26.0
@typescript-eslint/parser: 8.26.0

After updating, I ran npm run lint:eslint and encountered the following error:

šŸ‡®šŸ‡³India meghasharma

After running npm run lint:eslint, I still see the same warning that was present earlier:
Should we now check if we can update to a newer version of @typescript-eslint to resolve this?

šŸ‡®šŸ‡³India meghasharma

I have upgraded TypeScript to version 5.6.3 as per the discussion.
Verified with: npx tsc --version and npm list typescript
Ran npm install to ensure dependencies are correctly installed

šŸ‡®šŸ‡³India meghasharma

Yes @wim leers, This can happen when rect.height * canvasViewPortScale results in Infinity. This usually occurs if rect.height has an extremely large value or if canvasViewPortScale is not properly defined.

šŸ‡®šŸ‡³India meghasharma

Updated MR to resolve the console error for one and two column components. Let me know if any further adjustments are needed.
Thanks @omkar-pd

šŸ‡®šŸ‡³India meghasharma

I have updated the merge request to resolve the console error. Now, when we drag and drop a component in the canvas, no warning appears in the console. Please review and let me know if any further changes are needed. Thanks!

šŸ‡®šŸ‡³India meghasharma

I have reviewed the merge request after switching to the issue branch.
- The code now correctly separates FormElement and FormElementDescription from DrupalFormElement.tsx, making it cleaner and easier to reuse.
- The new components follow the existing structure and naming rules.
- Everything working as expected—labels, descriptions, errors, and attributes are handled properly.
Looks good to me!

šŸ‡®šŸ‡³India meghasharma

I am not able to reproduce the console error related to Drupal.announce.
I added an image component, selected an image from existing media uploads, and checked the console, but I didn't see any error.
This might have been fixed in another issue.

šŸ‡®šŸ‡³India meghasharma

I tested this issue locally and found that updating only ComponentOverlay.tsx resolved the warning. The issue occurred because an Infinity value was being assigned to the height CSS property. After modifying ComponentOverlay.tsx to ensure only finite and positive values using isFinite(), the warning no longer appears when dragging and dropping a component.

To fix this, update the style attribute at line number 261 in the ComponentOverlay.tsx file. This ensures rect.height * canvasViewPortScale is a valid finite number before assigning it to height.

style={{
  display: initialized ? '' : 'none',
  height: isFinite(rect.height * canvasViewPortScale) && rect.height * canvasViewPortScale > 0
    ? rect.height * canvasViewPortScale
    : 'auto',
  width: isFinite(rect.width * canvasViewPortScale) && rect.width * canvasViewPortScale > 0
    ? rect.width * canvasViewPortScale
    : 'auto',
  top: isFinite(elementOffset.verticalDistance * canvasViewPortScale)
    ? elementOffset.verticalDistance * canvasViewPortScale
    : 0,
  left: isFinite(elementOffset.horizontalDistance * canvasViewPortScale)
    ? elementOffset.horizontalDistance * canvasViewPortScale
    : 0,
}}
šŸ‡®šŸ‡³India meghasharma

I tested this issue on my local setup.
Before applying the fix, block names in the component list were encoded (e.g., Who's online block).
Some weird characters were appearing after "who" in the names.
After switching to the issue branch, I verified that the block names are now correctly decoded and displayed properly.
API Response: /xb/api/config/component now returns correctly decoded block names.
Example: "Who's new block" now appears correctly instead of the encoded version.
Everything seems to be working fine. Looks good from my side!

šŸ‡®šŸ‡³India meghasharma

Yes, children is a reserved prop in React, and using it as a slot name in Experience Builder is causing conflicts.
I am exploring this issue and trying to understand the best way to fix it. If anyone has suggestions on the correct approach for renaming the slot, I would like to learn more.

šŸ‡®šŸ‡³India meghasharma

I tested this issue on my local setup. Before checking out this branch, pressing Enter in the 'Add new code component' modal was not submitting the form, and I had to manually click the 'Add' button.
After switching to the issue branch, I verified that pressing Enter now successfully submits the form without needing to click the button.

šŸ‡®šŸ‡³India meghasharma

Hello @here, Commit is successfully removing the hyphens: auto from css & .pcss.css files. Looks good to me., So moving it to RTBC++. Thanks!.

šŸ‡®šŸ‡³India meghasharma

Added dark circle to active state when it's active as per the figma design.
Please review
Thanks!
Screenshot 2024-04-16 at 2.46.36 PM
Screenshot 2024-04-16 at 2.46.53 PM

šŸ‡®šŸ‡³India meghasharma

Megha Sharma
had the opportunity to volunteer on registeration desk
I have registered for most of the Attendee's. and I made goodie bags. I gave out welcome kits.

Production build 0.71.5 2024