perf: lazy load otrs section #4340
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frenautvh
requested review from
antleblanc,
varun257 and
radireddy
and removed request for
a team
frenautvh
commented
Jan 26, 2021
| angular.module(moduleName, ['oc.lazyLoad', 'ui.router']).config( | ||
| /* @ngInject */ ($stateProvider, $urlRouterProvider, coreConfigProvider) => { | ||
| const region = coreConfigProvider.getRegion(); | ||
| if (region === 'US') { |
There was a problem hiding this comment.
lazy load the module for US region otherwise perform a redirection
frenautvh
commented
Jan 26, 2021
| self.init = function init() { | ||
| // self.createMenu(); | ||
| self.isLoaded = true; | ||
| $compile(angular.element('#otrs-popup'))($rootScope.$new()); |
There was a problem hiding this comment.
added this line :
the otrs-popup directive code will only be loaded for US region, so we need to compile the directive (https://github.com/ovh/manager/pull/4340/files#diff-4eaf543ac34656361a5e0375c3730f87305031a7a866a005d4ce09d01183723dR134) after the otrs module have been lazy loaded
frenautvh
force-pushed
the
feat/dedicated-otrs-lazyloading
branch
from
98197d5 to
b29a0f2
Compare
frenautvh
commented
Jan 26, 2021
| @@ -291,7 +291,7 @@ <h1 data-translate="otrs_title"></h1> | |||
| </tbody> | |||
| <tbody data-ng-show="!loaders.tickets && tickets.ids.length"> | |||
| <tr | |||
| data-ng-repeat="ticket in tickets.detail | orderBy:'creationDate':true track by ticket.ticketNumber" | |||
| data-ng-repeat="ticket in tickets.detail | orderBy:'creationDate':true track by ticket.ticketNumber+$index" | |||
There was a problem hiding this comment.
added a fix here because there is a silent error in production (duplicate index in repeater)
cbourgois
previously approved these changes
Jan 26, 2021
JeremyDec
previously approved these changes
Jan 27, 2021
Base automatically changed from
feat/breadcrumb-billing-useraccount
to
develop
January 29, 2021 13:18
antleblanc
dismissed stale reviews from JeremyDec and cbourgois
The base branch was changed.
frenautvh
force-pushed
the
feat/dedicated-otrs-lazyloading
branch
from
b29a0f2 to
e400fe5
Compare
marie-j
reviewed
Feb 1, 2021
| return '/support'; | ||
| } | ||
| if (/^\/ticket\//.test(path)) { | ||
| return path.replace(/^\/ticket\//, '/support/tickets/'); |
There was a problem hiding this comment.
Maybe in a (near) future we could uniformize urls as it is quite confusing to know when it is supposed to be /ticket or /support/tickets
There was a problem hiding this comment.
yes i suppose it was introduced for backward compatibility with old urls, i wonder if it's still necessary
frenautvh
force-pushed
the
feat/dedicated-otrs-lazyloading
branch
from
e400fe5 to
f66634b
Compare
antleblanc
previously approved these changes
Feb 5, 2021
frenautvh
force-pushed
the
feat/dedicated-otrs-lazyloading
branch
3 times, most recently
from
90613ee to
a5cc6f1
Compare
Signed-off-by: frenauvh <[email protected]>
frenautvh
force-pushed
the
feat/dedicated-otrs-lazyloading
branch
from
a5cc6f1 to
24ce039
Compare
antleblanc
requested review from
JeremyDec,
cbourgois,
antleblanc and
marie-j
and removed request for
JeremyDec
antleblanc
approved these changes
Feb 5, 2021
antleblanc
changed the title
antleblanc
added
performance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
developDescription
Lazy load otrs section (part is conditionnally loaded only for US region)