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

page_title - don't require leading ": " in @page_title #3965

Merged
merged 3 commits into from
May 21, 2018
Merged

page_title - don't require leading ": " in @page_title #3965

merged 3 commits into from
May 21, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented May 21, 2018

Setting @page_title is a mechanism to provide custom <title> value.

For most of ops ui, this is computed from the current @layout, but that is not really pluggable and does not make sense for plugins.

So, we have @page_title and end up doing product_title + title - and calling it as productized_title(_(": Login")).


This updates productized_title to join the provided title with ": " automatically - so that we don't have to randomly @page_title = ": #{_('Migration')}" in v2v.

Instead, the line will be @page_title = _('Migration').

#3963: gaprindashvili yes because we need this for v2v to be able to use manageiq layouts instead of creating its own

Cc @mzazrivec , @martinpovolny

updates productized_title to join the provided title with ": " automatically

so that we don't have to randomly `@page_title = ": #{_('Migration')}"` in v2v
@miq-bot
Copy link
Member

miq-bot commented May 21, 2018

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/dd61f523966f62a871c0dc34ae135f5f770b16c8~...ebce29876f7b72926cc3517214d04db00abba84f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@@ -1,119 +1,122 @@
module ApplicationHelper
module Title
def productized_title(title)
product_title + title
if title.present?
"%{product}: %{title}" % {:product => product_title, :title => title}
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to i18n this string (the : basically), @mzazrivec ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say no for now. We can fix this later, should that be a problem in some locales.

@martinpovolny martinpovolny added this to the Sprint 86 Ending May 21, 2018 milestone May 21, 2018
@martinpovolny martinpovolny self-assigned this May 21, 2018
@martinpovolny martinpovolny merged commit 7eafd55 into ManageIQ:master May 21, 2018
@martinpovolny
Copy link
Member

@mzazrivec : are we getting updated translations for the Gaprindashvili releases? If not, we will need for this one.

@himdel himdel deleted the v2v-page-title branch May 21, 2018 14:31
@himdel himdel added the v2v label May 24, 2018
simaishi pushed a commit that referenced this pull request May 31, 2018
page_title - don't require leading ": " in @page_title
(cherry picked from commit 7eafd55)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit b4b293cd6d98755aff5293873fd5201378f3dc25
Author: Martin Povolny <mpovolny@redhat.com>
Date:   Mon May 21 15:53:29 2018 +0200

    Merge pull request #3965 from himdel/v2v-page-title
    
    page_title - don't require leading ": " in @page_title
    (cherry picked from commit 7eafd5522b3563d24518b6cc1ab531eeca58332f)

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

Successfully merging this pull request may close these issues.

5 participants