Maven Doxia Sitetools

List of site locales should be available in the rendering context.

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.1.4
  • Fix Version/s: 1.2
  • Component/s: Site renderer
  • Labels:
    None
  • Patch Submitted:
    Yes
  • Number of attachments :
    2

Description

Attached is a small improvement that adds a list of available site locales to the site rendering context. This would make a site template be "aware" of other locales available for the site and make it possible to implement a locale switching option.

Issue Links

is depended upon by

New Feature - A new feature of the product, which has yet to be developed. MSITE-41 Add a list of available language in site plugin

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Vincent Siveton added a comment -

sounds good

Show
Vincent Siveton added a comment - sounds good
Hide
Permalink
Yevgeny Nyden added a comment -

That's great. Someone with committer access, please, commit these changes.

Show
Yevgeny Nyden added a comment - That's great. Someone with committer access, please, commit these changes.
Hide
Permalink
Dennis Lundberg added a comment -

So, would the actual implementation of a "locale switcher" be done in the Velocity template (deafult-site.vm)?

Show
Dennis Lundberg added a comment - So, would the actual implementation of a "locale switcher" be done in the Velocity template (deafult-site.vm)?
Hide
Permalink
Yevgeny Nyden added a comment -

Yes. Additionally, I'd need to make this list of locales be available in the template rendering context (I guess, by modifying AbstractSiteRenderingMojo.java), and add additional css, javascript, and images.

Show
Yevgeny Nyden added a comment - Yes. Additionally, I'd need to make this list of locales be available in the template rendering context (I guess, by modifying AbstractSiteRenderingMojo.java), and add additional css, javascript, and images.
Hide
Permalink
Dennis Lundberg added a comment -

I had a look at your patch and have a question.

You have added this code:

    public void addSiteLocales(List locales) {
        siteLocales.addAll(locales);
    }

Wouldn't it be better to have this instead?

    public void setSiteLocales(List locales) {
        siteLocales = Collections.unmodifiableList( locales );
    }

When you work on implementing the "locale switcher", keep in mind that it's a bad idea to use images of flags to represent languages. You mentioned adding images in your previous comment.
See http://www.w3.org/TR/i18n-html-tech-lang/#ri20040808.173208643

Show
Dennis Lundberg added a comment - I had a look at your patch and have a question. You have added this code: public void addSiteLocales(List locales) { siteLocales.addAll(locales); } Wouldn't it be better to have this instead? public void setSiteLocales(List locales) { siteLocales = Collections.unmodifiableList( locales ); } When you work on implementing the "locale switcher", keep in mind that it's a bad idea to use images of flags to represent languages. You mentioned adding images in your previous comment. See http://www.w3.org/TR/i18n-html-tech-lang/#ri20040808.173208643
Hide
Permalink
Yevgeny Nyden added a comment -

Indeed, this would be better, but I think it wouldn't be consistent with the way we handle other collections in SiteRenderingContext. I made this collection immutable for the velocity's context. However, I don't feel strongly about this, and if you insist, I can change it. Btw, sorry about the style, I'm submitting a new patch with reformatted code.

As for the locale icons, I will answer on MSITE-41.

Show
Yevgeny Nyden added a comment - Indeed, this would be better, but I think it wouldn't be consistent with the way we handle other collections in SiteRenderingContext. I made this collection immutable for the velocity's context. However, I don't feel strongly about this, and if you insist, I can change it. Btw, sorry about the style, I'm submitting a new patch with reformatted code. As for the locale icons, I will answer on MSITE-41 .
Hide
Permalink
Yevgeny Nyden added a comment -

Here is an updated change.

Show
Yevgeny Nyden added a comment - Here is an updated change.
Hide
Permalink
Dennis Lundberg added a comment -

Patch applied in r1055222.
Thanks!

Show
Dennis Lundberg added a comment - Patch applied in r1055222 . Thanks!

People

  • Assignee:
    Dennis Lundberg
    Reporter:
    Yevgeny Nyden
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: