Greentube / localize-router

An implementation of routes localisation for Angular
MIT License
193 stars 93 forks source link

Unknown route switching language #140

Open quirinobrizi opened 6 years ago

quirinobrizi commented 6 years ago

Hi @meeroslav integrating your localize-router I found what I believe is an issue, correct me if I am wrong, what I am trying to do is that: when a user click on a new language the page is translated and the navigation is updated to march the new language, in summary the basic idea of this module.

The way the feature is implemented is that: I created 2 buttons and on click I invoke a function on my module that using the LocalizeRouterService#changeLanguage method set the new language.

HTML

<span class="pull-left ">
      <button style="padding-left: 0 !important; " class="nav-link d-inline " (click)="switchLanguage( 'en') ">
          English
      </button>
      <button style="padding-left: 0 !important; " class="nav-link d-inline " (click)="switchLanguage( 'it') ">
          Italian
      </button>
</span>

Component

@Component({
  selector: 'app-nav',
  templateUrl: './nav.component.html',
  styleUrls: ['./nav.component.css']
})
export class NavComponent implements OnInit {

  constructor(private localize: LocalizeRouterService) { }

  ngOnInit() {
    console.log('ROUTES', this.localize.parser.routes);
  }

  switchLanguage(language: string) {
    this.localize.changeLanguage(language);
  }

}

What I observe is this when I click the button the page is translated and the links are updated to the new language as well but when I try to click on any of the links the route is not recognized and the following error is thrown

Error: Cannot match any routes. URL Segment: 'it/howitworks'

doing some tests I can see that the error goes away and all work fine if after I click the button I manually change the language on the address bar and refresh the page with the new URL.

I spent few minutes debugging and I believe the issue is related to the LocalizeRouteParser#_translateRouteTree method that on line 163 of the master branch should invoke the LocalizeRouteParser#_translateProperty setting to true the 3rd prefixLang parameter. This is because, as far as my understanding goes, in this case the route will be prepended with the new language and as a result the routes configuration will be updated and back on LocalizeRouterService#changeLanguage the browser will be refreshed with the new route and as a result the system will start working correctly. I am not sure if there are other implications that is something that need to be considered.

Looking at the bug list I believe this is a duplicate of #139 but I'll leave this to you to judge, in the meantime I will update my switchLanguage method so that it refreshes the browser page with new language and update on this.

Thanks, ~Q

quirinobrizi commented 6 years ago

Hi @meeroslav done some more test and the only way that seem to work is to reload the full website so if I write something like that:

switchLanguage(language: string) {
    this.localize.changeLanguage(language);
    console.log('AFTER CHANGE ROUTES', this.localize.parser.routes);
    let currentUrl: string = this.router.url;
    let parts: Array<string> = /\/(.+)\/(.+)/.exec(currentUrl);
    if (parts) {
      let currentLanguage = parts[1];
      let currentPage = parts[2];
      if (currentLanguage != language) {
        let newUrl = `/${language}/${currentPage}`;
        // this.router.navigateByUrl(newUrl);
        window.location.href = newUrl;
      }
    } else {
      console.log('current URL does not match');
      window.location.href = `/${language}`;
    }
  }

it does work, the page is reloaded the new language is set and the navigation works with no problem. What I noticed and that may void what I wrote above but I don't know the module well enough to say so is that the redirect route is not updated when the the LocalizeRouterService#changeLanguage is invoked.

On Init

START ROUTES (2) [{…}, {…}] {children: Array(5), path: "en"} {path: "", redirectTo: "en", pathMatch: "full"}

After LocalizeRouterService#changeLanguage

AFTER CHANGE ROUTES (2) [{…}, {…}] {children: Array(5), path: "it"} {path: "", redirectTo: "en", pathMatch: "full"}

Thanks, ~Q

Elmanderrr commented 6 years ago

have the same problem, i think idea of this plugin is work exactly that your described in your issue +1 for fix this behavior

comoris commented 6 years ago

same problem here, +1

apolon23 commented 6 years ago

anyone solved the problem? @meeroslav

Elmanderrr commented 6 years ago

Solve it by manually changing the locale in the url and then reload the page, the easiest way changeLocale (locale) { window.location.href = this.router.url.replace(this.translateService.language, locale.code); }

quirinobrizi commented 6 years ago

@apolon23 my comment above provides the method I used to workaround the issue which in principle does what @Elmanderrr suggests as well.

quirinobrizi commented 5 years ago

So I got bored of seeing my app reloading as I consider it a very bad practice from technical and user experience point of view and decided to crack on figure the problem out, there are 2 issues that make this problem up:

So the solution is simple just reset the config of the router after the routes translation is completed so the changeLanguage should be implemented as

swtchLanguage(language: string, options: NavigationExtras, useNavigateMethod: Boolean) {
    if (language != this.parser.currentLang) {
      const currentUrl: string = this.router.url;
      const newRoute: string = this.parser.translateRoute(currentUrl.replace(this.parser.currentLang, language));
      this.parser.translateRoutes(language).subscribe(() => {
        // reset the router config so that routes are dynamically loaded.
        this.router.resetConfig(this.parser.routes);

        const extras = Object.assign({ skipLocationChange: false }, options);
        if (useNavigateMethod) {
          this.router.navigate([newRoute], extras);
        } else {
          this.router.navigateByUrl(newRoute, extras);
        }
      });
    }
  }

with the method above I can navigate my app without any page reload.

I am creating a little project that tries to provide a way of approaching multil-anguage applications, is still very young and based as you can understand on this library. In order to use this new implementation load MultilangModule as

const routes: Routes = [...]

@NgModule({
  imports: [
    TranslateModule.forRoot({
      loader: {
        provide: TranslateLoader,
        useFactory: (http: HttpClient) => { return new TranslateHttpLoader(http, '/assets/locales/', '.json'); },
        deps: [HttpClient]
      }
    }),
    MultilangModule.forRoot(routes, {
      parser: {
        provide: LocalizeParser,
        useFactory: (translate, location, settings, http) =>
          new LocalizeRouterHttpLoader(translate, location, settings, http),
        deps: [TranslateService, Location, LocalizeRouterSettings, HttpClient]
      }
    }),
    RouterModule.forRoot(routes)],
  exports: [TranslateModule, LocalizeRouterModule, RouterModule]
})
export class AppRoutingModule {

}

and use it as:

constructor(private multilangService: MultilangService) {}

...
switchLanguage(language: string) {
    this.multilangService.swtchLanguage(language, {}, true);
}

I will create a PR on this library as soon as I get time.

Hope the above can help.

@meeroslav I think we can considered this issue as verified, if you are OK with it I will use this issue to create a PR as soon as I get time.

Roelensdam commented 5 years ago

@quirinobrizi

I have the same issue. When changing language, the component reload (it occurs when hitting navigate or navigateByUrl in the 'changeLanguage` function)

The function : LocalizeRouterService.traverseRouteSnapshot will return a redundant url path like "fr/index/index" and then the router navigate to an unknown url => go back to home.

After fixing the function LocalizeRouterService.traverseRouteSnapshot to return a valid url like "fr/index". The component reload (ngOnInit is called) when a navigation occurs in the changeLanguage function :

this.router.resetConfig(this.parser.routes);
        if (useNavigateMethod) {
          this.router.navigate([url], extras);
        } else {
          this.router.navigateByUrl(url, extras);
        }

I've tried to implement your function switchLanguage() but i have the same bug : component is reloaded after navigation...

Any update on this ?

apolon23 commented 5 years ago

@Roelensdam i solved this problem like this: private localize: LocalizeRouterService onChange(language: string) { window.location.href = this.router.url.replace(this.localize.parser.currentLang, language); };

Roelensdam commented 5 years ago

@apolon23 Thanks for the tip, but i DON'T want to reload the page. This make the whole app reload. I DON'T want the component to be reloaded either.

Roelensdam commented 5 years ago

After further investigation, it seems linked to the immutability of the new router (Angular 6). When performing a resetConfig before a navigation, the component reload as his reference is different from the previous config.

Any suggestions, ideas, work around to prevent the reloading of the component ?

quirinobrizi commented 5 years ago

@Roelensdam I am not familiar with Angular 6, I am between moving and have no much time to look into it I will as soon as I find some time... sorry about that.

gilsdav commented 5 years ago

@Roelensdam Problem fixed on angular 6 fork https://www.npmjs.com/package/@gilsdav/ngx-translate-router

@Greentube you can check what changed here: https://github.com/gilsdav/ngx-translate-router/commit/de42217aeb349e3adc577a490bf6ddefb22e42f0

meeroslav commented 5 years ago

@quirinobrizi, @Roelensdam, @gilsdav,

This should be solved on both version v1.0.1 (for Angular 5.0.0-5.2.6) and 2.0.0-RC.1 (for Angular 6+).

Can you please verify?

Elmanderrr commented 5 years ago

@meeroslav it works, but still not like I expect in some cases 1) navigate to any page (/positions) 2) change language using changeLanguage method (eng) 3) navigate to any route (/users) 4) change language to previous (ru) 5) you will be automatically redirect to previous url (/positions)

gilsdav commented 5 years ago

@meeroslav Some bugs that are no more on my repo but still in 2.0.0-RC.1:

All probably fixed in this PR : https://github.com/gilsdav/ngx-translate-router/commit/de42217aeb349e3adc577a490bf6ddefb22e42f0

You can test by cloning my repo and you change every reference of my fork to localize-router into src folder (the demo app). Go to this path "http://localhost:4200/fr/testounet" and click on "change" button. We must come to "http://localhost:4200/en/mytest" and not to the home page.

meeroslav commented 5 years ago

@gilsdav, I like your approach with ReuseStrategy, but I would not make it part of localize-router. I will, however, add the link to the README. Based on the user's needs he/she can choose to have ngInit triggered on the lang change or not.

As for the empty subroute, I will look into it ... it's on my list.

meeroslav commented 5 years ago

@Elmanderrr, can you provide a minimal example with this behavior? Also, please open the new issue so I can target that specifically.

azhararmar commented 5 years ago

I was stuck with this issue when switching the route with parameters, such as listing-details/:id

When I use just the this.localize.changeLanguage(locale); for a url /ar/listing-details/7 it construct and redirect me to invalid URL en/listing-details%2F7;id=7, the expected URL redirect was /en/listing-details/7 which didn't happen.

Hence in my switch locale function, I decided to manually reconstruct the URL and navigate the user, here is the function I came up with, which is working fine for me.

import { LocalizeRouterService } from 'localize-router';
import { Router } from '@angular/router';

private localize: LocalizeRouterService,
private router: Router

const urlArray = this.router.url.split('/');
let switchToUrl = '/' + locale;
for (let i = 2; i < urlArray.length; i++) {
    switchToUrl += '/' + urlArray[i];
}
this.localize.changeLanguage(locale);
this.router.navigate([switchToUrl]);
azhararmar commented 5 years ago

I was excited it worked. Upon further testing it is still an issue, in some cases it still produces the same URL and in most cases it doesn't ...