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

Remove Comma #9117

Merged
merged 5 commits into from Feb 16, 2016
Merged

Remove Comma #9117

merged 5 commits into from Feb 16, 2016

Conversation

dgrammatiko
Copy link
Contributor

Remove comma if style is not set

See #9116

Remove comma if style is not set
@UncleR
Copy link

UncleR commented Feb 13, 2016

I have tested this item 🔴 unsuccessfully on 92ae51f

After chosing a module nothing happened. Returning to the editor was only possible with the close-Button of the selection window.


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

@richard67
Copy link
Member

I have tested this item 🔴 unsuccessfully on 92ae51f

Same as for @UncleR , it worked before this patch.

Following error is reported in browser console:

TypeError: jQuery(...).val(...) is undefined

if (jQuery("#extra_class").val().length)

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

See diverse tips on stackoverflow
@richard67
Copy link
Member

@dgt41 See https://github.com/dgt41/joomla-cms/pull/29 for the correction 😄


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

@richard67
Copy link
Member

@dgt41 Sorry, with my PR dgt41#29 mentioned above it also does not work. The Javascript error is solved, but the value of the extra class input field is never used with it. I try to find a solution but am neither a Javascript nor a jQuery expert.


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

The value of the extra class has to be checked inside the insert functions.
@richard67
Copy link
Member

@dgt41 I have just commited a change to my PR dgt41#29 for you, and now all works, just have tested. So if you accept my PR this PR here can be tested with success.


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

Check for empty jquery object before value length

Thanks @richard67
@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @richard67, @UncleR


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @richard67, @UncleR


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

@dgrammatiko
Copy link
Contributor Author

@richard67 thanks, of the patch, I've simplified these lines a bit more

@richard67
Copy link
Member

I have tested this item ✅ successfully on 29f9e6b

Tested with success.
Extra style parameter is added to the loadmodule or loadposition call at the end, separated by a comma, if set in the input box, otherwise if no extra style then no parameter and no comma added.
Examples:
{loadmodule mod_languages,Language switcher}
{loadmodule mod_languages,Language switcher,dada}
{loadposition position-0}
{loadposition position-0,dada}


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

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 29f9e6b

Everything works fine.

@UncleR
Copy link

UncleR commented Feb 14, 2016

I have tested this item ✅ successfully on 29f9e6b

OK.


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

@brianteeman
Copy link
Contributor

Thanks for testing - setting RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 15, 2016
@roland-d roland-d added this to the Joomla! 3.5.0 milestone Feb 16, 2016
roland-d added a commit that referenced this pull request Feb 16, 2016
@roland-d roland-d merged commit e594ecb into joomla:staging Feb 16, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 16, 2016
@dgrammatiko dgrammatiko deleted the patch-1 branch May 4, 2016 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants