Skip to content

Store dynamic variables into the JavaScript language store#6006

Closed
gunjanpatel wants to merge 3 commits intojoomla:stagingfrom
gunjanpatel:scriptVar
Closed

Store dynamic variables into the JavaScript language store#6006
gunjanpatel wants to merge 3 commits intojoomla:stagingfrom
gunjanpatel:scriptVar

Conversation

@gunjanpatel
Copy link
Copy Markdown
Contributor

@gunjanpatel gunjanpatel commented Feb 7, 2015

This patch will improve the functionality of Joomla.JText._() javascript function by supporting dynamic variable declaration easily.

For Example:

Let's try to get article title in javascript file.

  • In components/com_content/views/article/tmpl/default.php
JText::scriptVar('article', $this->item->title);
  • Use this variable in javscript file or declaration
jQuery(window).on('load',  function() {
    // Print in console
    console.log(Joomla.JText._('ARTICLE'));

    // Or try to alert the value
    alert(Joomla.JText._('ARTICLE'));
});

This will print article title in browser console.

Backwards compatibility

Full b/c, no problems are expected

Testing instructions

Try to follow the given example.

@bembelimen
Copy link
Copy Markdown
Contributor

bembelimen commented Feb 8, 2015

Why not using "normal" JS?

$js = 'Joomla.MyNamespace.article = "' . $this->item->title . '";';
JFactory::getDocument()->addScriptDeclaration($js);
alert(Joomla.MyNamespace.article);

I'm not sure about abusing language function for "normal" variables.

@gunjanpatel
Copy link
Copy Markdown
Contributor Author

gunjanpatel commented Feb 9, 2015

Reasons:

  1. Just imagine there are 20 of dynamic variable in your view source declared like this then output will look like,

    Joomla.MyNamespace.article = 'title';
Joomla.MyNamespace.article_name = 'name';
Joomla.MyNamespace.somethingelse = 'xyz';
... and so on

    When using this function it will just append one key and value in JSON string.

  2. We already have language store, this function is appending more string keys and values. Which will make it easy in use. You will just need JText::scriptVar($key, $value); in php file and use it anywhere in JS. Similarly, we already have JText::script(JText::_('SOME_LANGUAGE_CONSTANT));.

I didn't see any abuse of this function as it's using the same string store without disturbing JText::script functionality. :)

@compojoom
Copy link
Copy Markdown
Contributor

compojoom commented Mar 14, 2015

works as advertised, but I'm not sure that the JText class should be misused in this way.
JText is created for text and not for just storing anything, because it offers a json object.
I have the feeling that some developers will misuse this to store some configuration, instead of just use it for language constants.

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

@gunjanpatel
Copy link
Copy Markdown
Contributor Author

gunjanpatel commented Mar 16, 2015

Thanks for test @compojoom
I understand, but that is depends on developer it self. If developer is stupid enough to store password in json object then no body will able to stop him. This function is intended to give easier way to put something into PHP and get it easily into JS. So, I think developer must know what he should share in json. This function is not setting anything by it's own.

@anibalsanchez
Copy link
Copy Markdown
Contributor

anibalsanchez commented Jun 6, 2015

@text OK. Cool addition.

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

@anibalsanchez anibalsanchez mentioned this pull request Jun 6, 2015
Merged
@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Jun 6, 2015

Sorry, but I have to agree with @bembelimen here. This screams for misuse.

@chmst
Copy link
Copy Markdown
Contributor

chmst commented Jun 6, 2015

Please keep the Joomla Code as clean as possible. Abusing a class is always bad practice and will cause misunderstandings and errors in the future.

@anibalsanchez
Copy link
Copy Markdown
Contributor

anibalsanchez commented Jun 6, 2015

I have noticed a similar purpose in #3072

@gunjanpatel
Copy link
Copy Markdown
Contributor Author

gunjanpatel commented Jun 7, 2015

Thank you guys for review. It's better to close this PR then as per discussion it's going to abuse JText class.

@gunjanpatel gunjanpatel closed this Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@gunjanpatel @bembelimen @compojoom @anibalsanchez @Hackwar @chmst @roland-d @joomla-cms-bot