Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Fix tags creation #24743

Merged
merged 2 commits into from Apr 28, 2019
Merged

[4.0] Fix tags creation #24743

merged 2 commits into from Apr 28, 2019

Conversation

wilsonge
Copy link
Contributor

Summary of Changes

Currently the creation of dynamic tags is broken in the 4.0 branch. This fixes it (has been the case since we merged the update of choices 7)

Testing Instructions

Type a tag with a new value into the tags box and hit enter

Expected result

Tag is created

Actual result

It isn't

Documentation Changes Required

none

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 28, 2019
@@ -105,7 +105,7 @@ window.customElements.define('joomla-field-fancy-select', class extends HTMLElem
// Handle typing of custom term
if (this.allowCustom) {
this.addEventListener('keydown', (event) => {
if (event.keyCode !== this.keyCode.ENTER || event.target !== this.choicesInstance.input) {
if (event.keyCode !== this.keyCode.ENTER || event.target !== this.choicesInstance.input.element) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 108 exceeds the maximum line length of 100 max-len

@ghost
Copy link

ghost commented Apr 28, 2019

Are #23748 #23589 related Issues?

@richard67
Copy link
Member

I have tested this item ✅ successfully on 9ea050f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24743.

@ghazal
Copy link
Contributor

ghazal commented Apr 28, 2019

I have tested this item ✅ successfully on 9ea050f

>>This fixes it (has been the case since we merged the update of choices 7)
Not true. It never worked correctly since Joomla 4 used choices v 3.0.4.
And thanks to the great work of @bembelimen and @dgrammatiko on this script.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24743.

@ghost ghost removed NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 28, 2019
@ghost
Copy link

ghost commented Apr 28, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 28, 2019
@wilsonge
Copy link
Contributor Author

Not true. It never worked correctly since Joomla 4 used choices v 3.0.4.

it did because i tested downgrading the version and it worked ;)

@wilsonge
Copy link
Contributor Author

@franz-wohlkoenig it definitely doesn't fix the first issue and i can't reproduce the second (although as hannes suggested on the second it was related to setup doesn't mean it's fixed)

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 28, 2019
@@ -105,7 +105,8 @@ window.customElements.define('joomla-field-fancy-select', class extends HTMLElem
// Handle typing of custom term
if (this.allowCustom) {
this.addEventListener('keydown', (event) => {
if (event.keyCode !== this.keyCode.ENTER || event.target !== this.choicesInstance.input) {
if (event.keyCode !== this.keyCode.ENTER
|| event.target !== this.choicesInstance.input.element) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected tab character no-tabs

@ghost ghost removed NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 28, 2019
@ghost
Copy link

ghost commented Apr 28, 2019

Status set on Pending.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24743.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 28, 2019
@ghost ghost added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 28, 2019
@wilsonge
Copy link
Contributor Author

@franz-wohlkoenig just a codestyle fix for hound. No need to retest. As soon as tests pass i'm going to merge

@wilsonge wilsonge merged commit adfc28c into joomla:4.0-dev Apr 28, 2019
@wilsonge
Copy link
Contributor Author

Thanks guys!

@wilsonge wilsonge deleted the bug/tags-field branch April 28, 2019 14:08
@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants