Fix subjects/divisions being displayed in all languages after upgrade to 3.3.13 #306

Merged
merged 1 commit into from Apr 15, 2015

Conversation

Projects
None yet
4 participants
@drtjmb
Member

drtjmb commented Apr 14, 2015

This should fix #295 - apply the changes and regenerate subject/division view menus.

@sebastfr

This comment has been minimized.

Show comment
Hide comment
@sebastfr

sebastfr Apr 15, 2015

Contributor

you added a "if( false )" statement in Multilang ;-)

Contributor

sebastfr commented Apr 15, 2015

you added a "if( false )" statement in Multilang ;-)

@drtjmb

This comment has been minimized.

Show comment
Hide comment
@drtjmb

drtjmb Apr 15, 2015

Member

I know 😉

On 15 Apr 2015 9:01 am, Sébastien François notifications@github.com wrote:

you added a "if( false )" statement in Multilang ;-)


Reply to this email directly or view it on GitHubhttps://github.com/eprints/eprints/pull/306#issuecomment-93251463.

Member

drtjmb commented Apr 15, 2015

I know 😉

On 15 Apr 2015 9:01 am, Sébastien François notifications@github.com wrote:

you added a "if( false )" statement in Multilang ;-)


Reply to this email directly or view it on GitHubhttps://github.com/eprints/eprints/pull/306#issuecomment-93251463.

@patrickmcsweeney

This comment has been minimized.

Show comment
Hide comment
@patrickmcsweeney

patrickmcsweeney Apr 15, 2015

Collaborator

Yeah I dont get that... Why not just remove the condition and the else
statement entirely?

On Wed, Apr 15, 2015 at 9:22 AM, Timothy Miles-Board <
notifications@github.com> wrote:

I know [image: 😉]

On 15 Apr 2015 9:01 am, Sébastien François notifications@github.com
wrote:

you added a "if( false )" statement in Multilang ;-)


Reply to this email directly or view it on GitHub<
https://github.com/eprints/eprints/pull/306#issuecomment-93251463>.


Reply to this email directly or view it on GitHub
#306 (comment).

'But your intentions are beside the point, It's the outcome of your actions
that count...'

Collaborator

patrickmcsweeney commented Apr 15, 2015

Yeah I dont get that... Why not just remove the condition and the else
statement entirely?

On Wed, Apr 15, 2015 at 9:22 AM, Timothy Miles-Board <
notifications@github.com> wrote:

I know [image: 😉]

On 15 Apr 2015 9:01 am, Sébastien François notifications@github.com
wrote:

you added a "if( false )" statement in Multilang ;-)


Reply to this email directly or view it on GitHub<
https://github.com/eprints/eprints/pull/306#issuecomment-93251463>.


Reply to this email directly or view it on GitHub
#306 (comment).

'But your intentions are beside the point, It's the outcome of your actions
that count...'

@jiadiyao

This comment has been minimized.

Show comment
Hide comment
@jiadiyao

jiadiyao Apr 15, 2015

Contributor

Tim, I thought about this very fix before, but this means that render value for multilang would always render the first sub item (which may not be the item you want to render). Maybe this is ok because I cannot think of any other simple solutions!

Contributor

jiadiyao commented Apr 15, 2015

Tim, I thought about this very fix before, but this means that render value for multilang would always render the first sub item (which may not be the item you want to render). Maybe this is ok because I cannot think of any other simple solutions!

jiadiyao added a commit that referenced this pull request Apr 15, 2015

Merge pull request #306 from drtjmb/3.3
Fix subjects/divisions being displayed in all languages after upgrade to 3.3.13

@jiadiyao jiadiyao merged commit 04a2972 into eprints:3.3 Apr 15, 2015

@drtjmb

This comment has been minimized.

Show comment
Hide comment
@drtjmb

drtjmb Apr 15, 2015

Member

The choice seemed to be between doing something sensible (rendering the name part in the appropriate language) and rendering a grim looking table. I suggest the former be the default behaviour - remove the if(false) I added and just do the default - if someone wants to do something different they should just use the render_value property to override (and hopefully produce something a bit less grim :-)

Tim

On 15 Apr 2015 11:38 am, Jiadi Yao notifications@github.com wrote:

Tim, I thought about this very fix before, but this means that render value for multilang would always render the first sub item (which may not be the item you want to render). Maybe this is ok because I cannot think of any other alternative solution!


Reply to this email directly or view it on GitHubhttps://github.com/eprints/eprints/pull/306#issuecomment-93316051.

Member

drtjmb commented Apr 15, 2015

The choice seemed to be between doing something sensible (rendering the name part in the appropriate language) and rendering a grim looking table. I suggest the former be the default behaviour - remove the if(false) I added and just do the default - if someone wants to do something different they should just use the render_value property to override (and hopefully produce something a bit less grim :-)

Tim

On 15 Apr 2015 11:38 am, Jiadi Yao notifications@github.com wrote:

Tim, I thought about this very fix before, but this means that render value for multilang would always render the first sub item (which may not be the item you want to render). Maybe this is ok because I cannot think of any other alternative solution!


Reply to this email directly or view it on GitHubhttps://github.com/eprints/eprints/pull/306#issuecomment-93316051.

@drtjmb

This comment has been minimized.

Show comment
Hide comment
@drtjmb

drtjmb Apr 15, 2015

Member

@patrickmcsweeney - its a pull request, I wanted one of the core team to make that decision and modify it accordingly (I was just showing my working ;-)

Member

drtjmb commented Apr 15, 2015

@patrickmcsweeney - its a pull request, I wanted one of the core team to make that decision and modify it accordingly (I was just showing my working ;-)

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