Skip to content

Conversation

@bembelimen
Copy link
Contributor

Pull Request for Issue #21476 (comment).

Summary of Changes

Now one can use languages constants to have multilingual states

Testing Instructions

Create a state called "JPUBLISHED" and check it everywhere in the system (state list, article edit form, article list, transition edit form, ...)

Expected result

The language constant will be translated

Actual result

No translation at all

…-states-via-language-files

# Conflicts:
#	administrator/components/com_workflow/tmpl/states/default.php
#	administrator/components/com_workflow/tmpl/transitions/default.php
@bembelimen bembelimen changed the title Translate states via language files [4.0] Translate states via language files Aug 9, 2018
defined('_JEXEC') or die;

use Joomla\CMS\Language\Text;
use Joomla\CMS\Factory;
Copy link
Contributor

Choose a reason for hiding this comment

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

Factory already exists a few lines down. Adding it twice will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange, I thought I solved it with the conflict, thanks for the pointer!

<th scope="row">
<?php if ($canEdit) : ?>
<?php $editIcon = '<span class="fa fa-pencil-square mr-2" aria-hidden="true"></span>'; ?>
<a href="<?php echo $edit; ?>" title="<?php echo Text::_('JACTION_EDIT'); ?> <?php echo $this->escape(addslashes($item->title)); ?>">
Copy link
Member

Choose a reason for hiding this comment

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

need also a Text:: for the title

escape(addslashes($item->title)) to escape(addslashes(Text::_($item->title)))

@infograf768
Copy link
Member

Also:
same corrections should be done for Transitions
Using maybe the existing strings
JTOOLBAR_UNPUBLISH, etc. or new specific COM_WORKFLOW strings

To obtain (here for French. just modified for unpublished)

screen shot 2018-08-10 at 09 02 43

In both cases Modifier Non Publié for the title is not very nice phrasing. Maybe add a : between.

@bembelimen
Copy link
Contributor Author

@infograf768 I'll create a new PR for this, but thanks for the suggestion!

@infograf768
Copy link
Member

infograf768 commented Aug 13, 2018

Can conflicts be solved?
And obviously adapted to the new stage name instead of state

@bembelimen
Copy link
Contributor Author

Sure, I waited for #21566

@infograf768
Copy link
Member

I think that also Joomla! Default should be a string btw.

…-states-via-language-files

# Conflicts:
#	administrator/components/com_content/tmpl/articles/default.php
#	administrator/components/com_workflow/forms/transition.xml
#	administrator/components/com_workflow/tmpl/transitions/default.php
#	installation/sql/mysql/joomla.sql
#	libraries/src/Form/Field/TransitionField.php
#	libraries/src/Form/Field/WorkflowStageField.php
@bembelimen bembelimen changed the title [4.0] Translate states via language files [4.0] Translate stages via language files Aug 13, 2018
}

$workflowStages[$workflowStageKey][] = HTMLHelper::_('select.option', $stage->workflow_stage_id, $stage->workflow_stage_title);
$workflowStates[$workflowStateKey][] = HTMLHelper::_('select.option', $state->workflow_stage_id, Text::_($state->workflow_stage_title));
Copy link
Member

Choose a reason for hiding this comment

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

$state or $stage?

Copy link

Choose a reason for hiding this comment

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

Should be $stage

Copy link
Contributor Author

@bembelimen bembelimen Aug 17, 2018

Choose a reason for hiding this comment

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

Fixed

@infograf768
Copy link
Member

Also conflict with new file name.

@infograf768
Copy link
Member

And I foresee issues with #21520 as the sql does not use string constants.

use Joomla\CMS\Factory;
use Joomla\CMS\Form\FormHelper;
use Joomla\CMS\Workflow\Workflow;
use Joomla\CMS\Form\Field\ListField;
Copy link

Choose a reason for hiding this comment

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

You don't need to import this as it's in the same namespace

$user = Factory::getUser();
$userId = $user->id;

$lang = Factory::getLanguage();
Copy link

Choose a reason for hiding this comment

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

You aren't using this property

Copy link

@Joomv Joomv left a comment

Choose a reason for hiding this comment

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

Also requires conflicts fixing as well as comments

Deleted useless hashtag here... (JM)

@infograf768
Copy link
Member

infograf768 commented Aug 23, 2018

Missing Text:: line 118 of tmpl/transitions/default.php to get
<a href="<?php echo $edit; ?>" title="<?php echo Text::_('JACTION_EDIT'); ?> <?php echo Text::_($this->escape(addslashes($item->title))); ?>">

Otherwise good first step for me

<?php endif; ?>
</div>
<div class="mr-auto"><?php echo $this->escape($item->stage_title); ?></div>
<div class="mr-auto"><?php echo Text::_($this->escape($item->stage_title)); ?></div>
Copy link
Member

Choose a reason for hiding this comment

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

I think correct would be do escape() after translation, not before 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, thx

@ghost
Copy link

ghost commented Aug 24, 2018

I have tested this item ✅ successfully on 9d0ba73


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

@ghost
Copy link

ghost commented Aug 24, 2018

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 24, 2018
@alikon
Copy link
Contributor

alikon commented Aug 24, 2018

https://github.com/joomla/joomla-cms/pull/21506/files#diff-135c6f58583408312a709459b19594c7
it lacks the upgrade path and obviously postgresql

done in bembelimen#22

@alikon
Copy link
Contributor

alikon commented Aug 24, 2018

question:
should we do the same on the #__workflow_transitions ?

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 24, 2018
@ghost
Copy link

ghost commented Aug 24, 2018

Status back on "Pending".

@infograf768
Copy link
Member

infograf768 commented Aug 24, 2018

@alikon
I thought we could do that after the follow-up on this.
I explain:

We also need to translate the transitions. I.e. enter in db some string for the actions
Unpublish, Publish, etc.
Then translate the titles in transitions Manager with something like

										<?php echo $editIcon; ?><?php echo Text::_($item->title); ?>
											</a>
										<?php else: ?>
											<?php echo Text::_($item->title); ?>
										<?php endif; ?>

so basically we would have something like

INSERT INTO `#__workflow_transitions` (`id`, `asset_id`, `published`, `ordering`, `workflow_id`, `title`, `description`, `from_stage_id`, `to_stage_id`) VALUES
(1, 0, 1, 1, 1, 'JUNPUBLISH', '', -1, 1),
(2, 0, 1, 2, 1, 'JPUBLISH', '', -1, 2),
(3, 0, 1, 3, 1, 'JTRASH', '', -1, 3),
(4, 0, 1, 4, 1, 'JARCHIVE', '', -1, 4);

And add the new lang strings.
Is it worth changing now db or do it in the next PR?

@infograf768
Copy link
Member

Or do it in this PR?

@alikon
Copy link
Contributor

alikon commented Aug 24, 2018 via email

@infograf768
Copy link
Member

@bembelimen
Your decision. If you do transitions in next PR, let's not forget @alikon necessary updated patch.

@bembelimen
Copy link
Contributor Author

Thanks @alikon for the PR

I would do the translation in a new PR, so we have small code changes.

@infograf768
Copy link
Member

Drone error is unrelated to this PR.

@wilsonge Please merge.

@infograf768
Copy link
Member

rtc


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 25, 2018
@brianteeman
Copy link
Contributor

Whoever merges can you directly fix the drone error

@wilsonge wilsonge merged commit 4314e47 into joomla:4.0-dev Aug 31, 2018
@wilsonge wilsonge added this to the Joomla 4.0 milestone Aug 31, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 31, 2018
@wilsonge
Copy link
Contributor

Thanks guys!

@bembelimen bembelimen deleted the translate-states-via-language-files branch September 10, 2018 13:50
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.

8 participants