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

[WIP] prefix_none strategy #165

Closed
wants to merge 1 commit into from

Conversation

appinteractive
Copy link

@appinteractive appinteractive commented Dec 7, 2018

I need to be able to switch the language without a prefix as the locale is only used for the interface, not for the content (which can be multilingual).

Therefore I startet to add a PREFIX_NONE strategy.

I would be happy for some feedback

Problems to solve:

  • do not prefix links
  • fix locale switch (currenty does not work on the root route /)
  • write tests for the PREFIX_NONE strategy (found in test/fixture/prefix_none/)
  • more link tests
  • clean up code

How to test

  • in browser: node test/fixtures/prefix_none/start.js or node test/fixtures/basic/start.js
  • run tests: yarn jest test/fixtures/prefix_none or yarn jest test/fixtures/basic

if (routePathLang && routePathLang.length === 2) {
console.log('REDIRECTING TO ' + (route.path.slice(3) || '/'))
locale = routePathLang
app.i18n.switchLocale(locale)

Choose a reason for hiding this comment

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

"app.i18n.switchLocale is not a function"

Copy link
Author

@appinteractive appinteractive Dec 7, 2018

Choose a reason for hiding this comment

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

🤔 hmm that could be explain why it does not switch on the root route

Thsks @Magnum5234 for pointing that out!

Choose a reason for hiding this comment

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

2018-12-07 16_30_22-start
I recommend using an IDE that is highlighting such errors in advance :)

Copy link
Author

Choose a reason for hiding this comment

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

@Magnum5234 which editor are you using?

Copy link

@Magnum5234 Magnum5234 Dec 7, 2018

Choose a reason for hiding this comment

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

I use for all web related stuff PHP Storm
https://www.jetbrains.com/phpstorm/

@Magnum5234
Copy link

When I remove the faulty line above, my cookie settings are not longer respected. Cookie is no longer stored when I visit the site. I will be stored however after the change. Also my strategy is no longer followed (language not longer applied to path). A short while after, the page breaks completely until I remove the language cookie again. That might be due to routing broken as the language path are no longer valid. (/en/something became /something and stopped working)
Maybe try my settings and see for yourself.

I run my test project with:

      {
        locales: [
          {
            code: 'de',
            iso: 'de-DE',
            file: 'de-DE.js'
          },
          {
            code: 'en',
            iso: 'en-GB',
            file: 'en-GB.js'
          }
        ],
        detectBrowserLanguage: {
          useCookie: true,
          alwaysRedirect: true,
          fallbackLocale: 'en'
        },
        strategy: 'prefix_except_default',
        defaultLocale: 'de',
        vueI18n: {
          fallbackLocale: 'de'
        },
        lazy: true,
        langDir: 'lang/',
        parsePages: false,
        pages: pageLanguages
      }

@appinteractive
Copy link
Author

So I guess we need more tests.

@appinteractive appinteractive changed the title [WIP] added prefix_none strategy and tests [WIP] prefix_none strategy Dec 17, 2018
@JakubKoralewski
Copy link

What seems to currently be the problem? This PR kind of died. I'd love to help.

@appinteractive
Copy link
Author

The problem is that the current strategies do not fit for web apps with multi lang content, only websites. As for us the Lang setting only effects the ui language, not the content we don’t want to have any reference to the current Lang in the url and therefore need to switch the Lang also in place without switching the url.

All of that is currently not possible and turned out to be very error prone in the current implementation as the tests also are more of an decoration then really useful atm.

We gave up and implemented it on our own which turned out to be very simple. So at the Moment that PR is indeed dead but feel free to take it further if it is useful also for your case. Would be nice if the defacto standard community plugin would also work for multi Lang content sites.

@paulgv
Copy link
Collaborator

paulgv commented Apr 25, 2019

This PR has been inactive for a while, closing it for now.

Hopefully, someone can pick this up, it would certainly be a nice feature to have.

@paulgv paulgv closed this Apr 25, 2019
farnabaz added a commit to farnabaz/i18n-module that referenced this pull request Mar 25, 2021
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.

None yet

4 participants