-
Notifications
You must be signed in to change notification settings - Fork 19
Introduce search:input event to select, deprecate autocomplete and add OcRecipient #1521
Conversation
License/CLA fails since @paulcod3 has changed his accounts and can't sign in his old account (from which the commit has been cherrypicked). Perhaps best to reset the changes locally and change authors from his old to his new account |
33d8d01
to
ef1d093
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love a lot of what I'm seeing here, small questions about broken/outdated examples and we should be good to go!
205f7e0
to
f851e2b
Compare
f851e2b
to
781e67c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running yarn start
I get a lot of deprecation warnings like these DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.
, while master looks fine. Can you confirm?
Yep, some deps have been updated (no major version bump though). As it doesn't break yet anything, I would say let's take care of updating to new functions in a separate PR... |
Some of them are btw coming from the UIkit... since we're not updating that one anymore due to some breaking changes it might be necessary to either not update Dart Sass to v2, change the decision and invest the effort to update UIkit even though we're going to remove it or finally move forward with sending it into not-in-our-project dependency heaven 🤷 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have validation for the format of the icon
inside the recipient. And the deprecation warnings of the updated dependencies need to be fixed in a separate PR. Rest looks good to me 👍 💪
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👥
OcSearch can be used with prop
:multiple="true"
to achieve the same behaviour as our autocomplete and even better with including selected options directly in the select...