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

Restore internationalization functionality in toolbars #10787

Merged
merged 1 commit into from Aug 30, 2016

Conversation

Projects
None yet
4 participants
@mzazrivec
Contributor

mzazrivec commented Aug 25, 2016

Broken since #9753

Fixes: #10748

@himdel

This comment has been minimized.

Show comment
Hide comment
@himdel

himdel Aug 28, 2016

Contributor

(Test failure should be resolved now, as of #10813)

Contributor

himdel commented Aug 28, 2016

(Test failure should be resolved now, as of #10813)

@himdel

This comment has been minimized.

Show comment
Hide comment
@himdel

himdel Aug 29, 2016

Contributor

Hmm, why does this fix that issue? Seems like it shouldn't affect anything but i18n..

Either way, re-kicked the test and hoping :)

Though, I'm not quite sure this is the right place, seems like this should be called when generating the toolbar - which is either already being done in toolbar_helper, or that file is dead and it is done on the angular side .. but then again, buttons_to_html is still called from javascript_pf_toolbar_reload.. so maybe the conversion is not quite complete.. looking into it :) (EDIT: aaaah, that's being addressed in #10827)

Contributor

himdel commented Aug 29, 2016

Hmm, why does this fix that issue? Seems like it shouldn't affect anything but i18n..

Either way, re-kicked the test and hoping :)

Though, I'm not quite sure this is the right place, seems like this should be called when generating the toolbar - which is either already being done in toolbar_helper, or that file is dead and it is done on the angular side .. but then again, buttons_to_html is still called from javascript_pf_toolbar_reload.. so maybe the conversion is not quite complete.. looking into it :) (EDIT: aaaah, that's being addressed in #10827)

@martinpovolny

This comment has been minimized.

Show comment
Hide comment
@martinpovolny

martinpovolny Aug 30, 2016

Contributor

Code looks good.

toolbar_helper has to go or all the toolbar rendering routines can be moved there from application_helper.

@mzazrivec : please, rebase and fix tests so that I can test this in the UI.

Contributor

martinpovolny commented Aug 30, 2016

Code looks good.

toolbar_helper has to go or all the toolbar rendering routines can be moved there from application_helper.

@mzazrivec : please, rebase and fix tests so that I can test this in the UI.

@miq-bot

This comment has been minimized.

Show comment
Hide comment
@miq-bot

miq-bot Aug 30, 2016

Member

Checked commit mzazrivec@b43ca55 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. 🍰

Member

miq-bot commented Aug 30, 2016

Checked commit mzazrivec@b43ca55 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. 🍰

@mzazrivec

This comment has been minimized.

Show comment
Hide comment
@mzazrivec

mzazrivec Aug 30, 2016

Contributor

@martinpovolny Rebased & fixed tests.

Contributor

mzazrivec commented Aug 30, 2016

@martinpovolny Rebased & fixed tests.

@martinpovolny

This comment has been minimized.

Show comment
Hide comment
@martinpovolny

martinpovolny Aug 30, 2016

Contributor

code looks good, tested in the UI

Contributor

martinpovolny commented Aug 30, 2016

code looks good, tested in the UI

@martinpovolny martinpovolny merged commit b3d92e3 into ManageIQ:master Aug 30, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-4.5%) to 32.124%
Details

@mzazrivec mzazrivec deleted the mzazrivec:restore_i18n_in_toolbars branch Sep 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment