This makes sense. The ordered list number 4 needs to be updated to 3.
@plopesc, thanks for your work on this.
Your test correctly failed prior to the fix, and passes after the fix. I've refactored a bit, removing the AJAX adjustments and adjusted the conditional logic for persisting the country selection. Feel free to add an issue if you'd like to describe steps to reproduce the AJAX issue your team is experiencing.
MR !14 is ready for review. Will leave this open for a couple days to receive feedback, but this looks good to me.
I'd imagine that, until this is added to Radix, in order to adopt
Storybook →
, we'd just have to create our own *.stories.twig
files in our sub-theme, and for those, a *.compentent.yml
with the replaces
key pointing to the target component in Radix. Anything else I may be missing?
Added ✨ Bump supported intl-tel-input version Active . Seems appropriate to get the front-end package dependency a good solid update before a major release.
When 2.0.0-rc2
gets released, it will introduce breaking changes (changes that require consuming applications to make code modifications to continue working correctly with the new version). While release candidates may be considered close to stable, this update is long overdue, and introducing breaking changes on a release candidate isn't strictly forbidden. We'll be sure to communicate this in the release documentation for RC2.
@plopesc pointed out that this feature may already be addressed by Telephone Validation → . I wonder if @omkar-pd might be willing to share reprucible steps if this becomes an issue with the modules not playing nicely.
Removed 🐛 Fix validation not being applied Active as a release blocker. Willing to consider putting it back in. Awaiting community feedback.
I realized that this should be marked as a feature request, not a bug. To fail validation and prevent save might even be considered a breaking change. Though, if the community wants this, we could add it to a release candidate before making it to 2.0.0. Perhaps even make it configurable.
Moving this out of 📌 Release 2.0.0 Needs review , unless someone objects.
In your video, you type the exit code and country code into the field, rather than select the flag. I was able to reproduce this bug by selecting the flag.
I was able to reproduce this issue. Digging.
Need to verify this indeed has been fixed.
I created this ticket based on #3302638-3: Telephone International Widget 2.0.0 stable release plan → . I assumed the front-end validation from 🐛 Doesn't work with multiple phone fields Needs work was sufficient, but I realize now that this is referring to back-end validation.
Absolutely! Thanks for calling that out, @johnpicozzi. I'll take a moment to look through the other tickets which may have influenced the total sum of changes captured here. Thanks everyone for your patches, MRs, and review.
This is the last holdout for a rc2
and
📌
Release 2.0.0
Needs review
.
The composer.libraries.json
file has been removed in
🐛
Doesn't work with multiple phone fields
Needs work
.
Captured in 🐛 Doesn't work with multiple phone fields Needs work . Thank you for this find!
As part of the work done in
🐛
Doesn't work with multiple phone fields
Needs work
, we load the utils.js
as part of the *.libraries.yml
.
The composer.libraries.json
has been removed as part of
🐛
Doesn't work with multiple phone fields
Needs work
. See that ticket for details.
Fixed with 🐛 Doesn't work with multiple phone fields Needs work .
@kerasai, I was still seeing this issue during my tests of 🐛 Doesn't work with multiple phone fields Needs work .
Got a test added and refactored the work from the above patches and MR !8. Good job and thanks , everyone.
Please, someone review MR !9.
Never mind. That intlTelInputWithUtils.js
doesn't even come with the package. Must be outdated docs.
Option 1: intlTelInputWithUtils
If you're not concerned about filesize (e.g. you're lazy loading this script), the easiest thing to do is to just use the full bundle (/build/js/intlTelInputWithUtils.js), which comes with the utils script included. This script can be used exactly like the main intlTelInput.js - so it can either be loaded directly onto the page (which defines window.intlTelInput like usual), or it can be imported like so: import intlTelInput from "intl-tel-input/intlTelInputWithUtils".
-- https://github.com/jackocnr/intl-tel-input?tab=readme-ov-file#loading-th...
Given this is loaded when the widget is used, let's adopt the above implementation in lieu of the changes made from #11.
Merged 📌 Remove the composer libraries workaround Active . This ticket can be updated to reflect any desired changes.
@omkar-pd, #3143446: Flag not selected on node editing form forces user to re-select → and 🐛 Fix validation not being applied Active capture the concerns noted.
Apologies for the crazy number of commits attempting just to get a working test. I had a hell of a time because I didn't think to ensure the test user was logged in to view the node/add page to test the field widget. oi vey.
But, the really cool thing is, I got a GitLab CI customization and browser tests (inspired by DropzoneJS) implemented!
This CI composer job and testing nicely covers the exact instructions now provided in the readme and ensures the widget gets the intl-tel-input
flag select in place.
Would appreciate your insight on the approach done in 📌 Remove the composer libraries workaround Active .
Because Drupal prefers third-party libraries to be accessible from docroot (web server), the convention is for packages of type drupal-library
to be installed to the /libraries/
folder. This is also the place where Drupal is able to allow and manage multiple versions of packages, as dependency management for a modular project via the vendor/
directory could get unwieldy.
So, the approach in the above-mentioned ticket seems the best option, and a good replacement for the composer.libraries.json
and asset-packagist
options of old.
With this release, it will include README changes that will need to be published to the project page.
See 📌 Remove the composer libraries workaround Active .
Nice. Good to see you even made changes based on eslint in the CI pipeline.
Leave MR !11 as-is now. I am working on 🐛 Doesn't work with multiple phone fields Needs work where there will be changes to the CI and test suite we need to capture before this can have tests added.
This is a big undertaking. While I do believe it is ideal to tackle issues distinctly, the frequency of updates to this module has made it such that a big change affecting multiple bugs is inevitable. I will review this further. I plan to add tests as described in #10. Thanks, everyone.
I think I see what happened. It seems Ahmad's #17 patch mistakenly missed somethings, #18 is a re-roll of #12 with his desired change to INTERNATIONAL format rather than E164.
Needs the patch rolled into an MR. Needs a regression test.
If unable to reproduce, we can mark this closed as works as designed. Will keep this open for a couple weeks for feedback.
Already Drupal 10 compatible. Rector appropriately fixes the public property, and submitted a patch for it to be protected. This same fix will come along with the Drupal 11 automated ticket.
Does the patch in #3302639: Allow GET parameters in the shorten URL → solve this?
I pushed a new branch to the fork: 3349389-image-block-and-media-linkable.
There I attempt to introduce imageBlock
and media
as linkable models. I got about as far as the functionality introduced by !28--attributes stick, but not the target="_blank"
.
However, I think I now understand the reasoning behind branch 3349389-link-attributes-dont
and suggest work continues there. I've learned that the htmlLinkAttributes
overrides skip attempting to target specific models (e.g. imageBlock
, media
).
The core trick in the patch is that it doesn’t try to manually wrap images and media in tags itself—instead it piggy-backs on Drupal’s existing “HTML Support” converter that already knows how to take a
htmlLinkAttributes
-model attribute on an element (like a block image or media embed) and turn it into a real wrapper, and vice versa.
I'm just still not sure why the target attribute isn't being handled given this explanation.
Rebased Merge request !28 to capture recent slew of updates to 2.2.x.
Though this captures the work in progress so far, the issue has not been resolved. Still needs work. Patch #31 likely needs a re-roll to apply correctly, but will wait until we have created a working solution.
No merge required. It turns out that, although the 2.2.x to 2.x comparison shows the logo change as a difference, the logo change is indeed part of 2.2.x.
The 2.2.x is the most up-to-date branch.
We can safely rebase 2.x from 2.2.x, then remove 2.2.x.
Maintainer may find the following helpful.
git fetch --all
git checkout --track origin/2.x
# skipped previously applied commit 8fb5e79, reapply-cherry-picks to include skipped commits.
git rebase --reapply-cherry-picks 2.2.x
# see that the branches now match at HEAD.
git log
# delete remote branch.
git push origin :2.2.x
# delete local branch.
git branch -D 2.2.x
This is a follow-up to #3371633-40: Exceptions thrown when toggling Advanced section of linking form → .
We no longer need to wait to decide if the other ticket takes priority and fixes this issue. Given the recent slew of changes merged to 2.2.x, and possibly due specifically to ✨ Use built-in CollapsibleView instead of DetailsView Active , this issue seems to have been resolved.
Marking fixed. Feel free to reopen if you're able to reproduce the exceptions described above. Note: this ticket will automatically be closed (fixed) if no status change is made in 2 weeks.
I've opened 🐛 Fix keyboard accessibility of advanced attribute fields Active for the issue described in #38.
Given the recent slew of changes merged to 2.2.x, and possibly due specifically to [#16061370], this issue seems to have been resolved.
Marking fixed. Feel free to reopen if you're able to reproduce the exceptions described above. Note: this ticket will automatically be closed (fixed) if no status change is made in 2 weeks.
You may need to run Yarn's yarn dedupe command to potentially consolidate the package to a single version if the versions are compatible.
This introduced duplicate, higher version than core in yarn.lock
file:
"@ckeditor/ckeditor5-core@41.3.1":
version "41.3.1"
resolved "https://registry.yarnpkg.com/@ckeditor/ckeditor5-core/-/ckeditor5-core-41.3.1.tgz#b4bf65cc1a291d78d9e47269ae159aec1451d99b"
integrity sha512-h+PgPtCpS2vjO3HbKMYtddRPW+B3AJx9qpixmHJnUZMiFCmRjUZjXATjpi3j+kSQISs4L2Yghq+lsAQxyGHb+A==
dependencies:
"@ckeditor/ckeditor5-engine" "41.3.1"
"@ckeditor/ckeditor5-utils" "41.3.1"
lodash-es "4.17.21"
"@ckeditor/ckeditor5-core@41.4.2":
version "41.4.2"
resolved "https://registry.yarnpkg.com/@ckeditor/ckeditor5-core/-/ckeditor5-core-41.4.2.tgz#0779a727b0238368fc09ee4ec855536ea1ab24db"
integrity sha512-kCIJjviiMNIMBMx7XFXFp1IeTELQKv7xyPJiVFDyUftIfthf9uWty72ipZ3BBNBGBkaoTiSzDZ507EsX6czuIQ==
dependencies:
"@ckeditor/ckeditor5-engine" "41.4.2"
"@ckeditor/ckeditor5-utils" "41.4.2"
lodash-es "4.17.21"
Perhaps, to ensure we match core, we should remove the open constraint (~) on ckeditor5.
✨
Use built-in CollapsibleView instead of DetailsView
Active
introduced a bug to 2.2.x
: the tabindex problem experienced in !14, described in #28--keyboard navigation skips advanced fields.
Do we want a new issue for this, or will we tackle that here?
Can we add the following bash examples to help with Yarn linking all packages?
<code>
cd path/to/ckeditor5/packages
for dir in ./*; do (cd "$dir" && yarn link); done
cd path/to/core
for dir in /path/to/ckeditor5/packages/*/; do (dirname=$(basename "$dir") && yarn link "@ckeditor/$dirname"); done
</code>
I am attempting to debug this via a source build. Requires a bump to CKEditor5 v41.3.1 due to errors mentioned in ✨ Upgrade CKEditor to match Drupal core at 41.3.1 Active .
Running through
CKEditor 5 development →
docs, when I am on v35.1.0, the yarn install
command fails with:
error @es-joy/jsdoccomment@0.36.1: The engine "node" is incompatible with this module. Expected version "^14 || ^16 || ^17 || ^18 || ^19". Got "22.14.0"
When I am on 41.3.1, this error does not occur.
If this really is a necessary component of the CKEditor Plugin plugin module development, is it possible to provide the Drupal GitLab CI with an NPM build so these artifacts are not required to be committed?
Moved to the source example project driving Drupal modules to use webpack builds.
As I learn more about CKEditor 5 Plugin plugin development, I understand now that projects like Editor Advanced Link are led to implement a build by this project.
I am curious what, if any, dependency issues exist that might prevent a Drupal module from not including a build process for CKEditor Plugin plugins.
Re-titled this issue to be more clear.