-
Notifications
You must be signed in to change notification settings - Fork 29
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
Feature/optimize css #285
Feature/optimize css #285
Conversation
surfermicha
commented
Mar 24, 2021
- Reopened the PR after renaming this branch following to git flow conventions.
- Changed the base branch to merge to develop according to git flow as well
* removed hardcoded URL * Removed unnecessary endpoint
@sdibernardo I altered your PR. Could you please pull this new branch and fix the issues in frrontend? |
@surfermicha Thanks for adjusting the PR to git flow. In the meantime I pushed the fix for the front end checks |
Thanks @sdibernardo! I have tested it locally and it works like a charm! 👍🏻 It's much faster now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing your changed was successful. I'm just wondering about the routing file. There might be an issue.
@@ -19,6 +19,11 @@ const appRoutes: Routes = [ | |||
{path: '', component: MainLayoutComponent, canActivateChild: [AuthenticationGuard], children: [ | |||
{path: '', redirectTo: RoutingUrls.myAvatar, pathMatch: 'full'}, | |||
{path: RoutingUrls.start, component: StartPageComponent}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RoutingUrls.start
path is covered twice, so that your entry wouldn't be reached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is just a suggestion to use lazy loading and not meant as a proper "fix". Additionally lazy loading just one component doesnt make sense here, so unless all components are loaded lazily, I suggest to leave this PR open.
I try to add the missing routes unless someone else wants to :D
I did a quick review - this looks good @sdibernardo ! @surfermicha if you agree, I would merge this. |
@Christoph-Meyer, as @sdibernardo mentioned above, we should wait with merging until the whole routing is configured to be lazy loaded |
I added the missing main components 🎉 |
I'm sorry, I missed the recent changes. I will test it locally and continue the review afterwards. |
* Add branching section into CONTRIBUTING.md - Merge in develop (#286) * Clean up (#282) * removed hardcoded URL * Removed unnecessary endpoint * Add illustrations for feature and hotfix branches to use in CONTRIBUTING.md Image source: https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow. License checked * Add branching section into to use in CONTRIBUTING.md Describe the use of Gitflow in SonarQuest Co-authored-by: Michael Landreh <[email protected]> * Fix #270 - Password is returned (#287) * Change input color of login input fields * Add necessary fields into UserDto * Fix Nullpointer Exceptions in world service This occurs when a world should be found by an id that is null * Change return type of 'getUser' and 'users' into UserDto * Adjust getUser to be null safe * Fixed Issue#290 * 0.8.6-SNAPSHOT * Sonarlint Cleanup * Sonarlint Cleanup * Feature/optimize css (#285) * Clean up (#282) * removed hardcoded URL * Removed unnecessary endpoint * Paar kleine Aenderungen für potentielle Optimierungen * Hopefully fixed package-lock.json to fit regular package file * Added the missing main components * WIP Co-authored-by: Sascha <[email protected]> * Version der ajv-Dependency geändert. * package-lock nachgepflegt. * Node Version im GitHub-Action-Skript angepasst. * Weitere Anpassungen an der Github-Action * Configuration für Produktion angepasst * Node Version nochmal angepasst * Änderungen teilweise zurück gerollt und Abhängigkeiten ausgebaut. --------- Co-authored-by: Nils <[email protected]> Co-authored-by: Michael Landreh <[email protected]> Co-authored-by: MeC <[email protected]> Co-authored-by: Sascha <[email protected]>