-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[GSoC'24] M1.3 - Modify translations frontend modal #20439
Changes from all commits
30cb556
f9ed166
a0eb666
80f2b9c
ed19d3f
020b0c9
4e32851
44e07a7
e59dd79
ff26a22
597ebb3
2e9e668
8dedc2a
92ba7b2
837e723
793ef8a
d92ea35
17e57ca
dbb7fca
c8ac939
1361475
ffbbb33
bfa068b
17e050c
690499b
2d5bca3
f835f94
cf85be6
df70958
e9fd519
5f1fcbc
9401aa2
64611c8
196b344
4553490
13d3fc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,10 @@ | |
|
||
import {Component, Input} from '@angular/core'; | ||
import {NgbActiveModal} from '@ng-bootstrap/ng-bootstrap'; | ||
import {NgbModal} from '@ng-bootstrap/ng-bootstrap'; | ||
import {ConfirmOrCancelModal} from 'components/common-layout-directives/common-elements/confirm-or-cancel-modal.component'; | ||
import {PlatformFeatureService} from 'services/platform-feature.service'; | ||
import {ModifyTranslationsModalComponent} from 'pages/exploration-editor-page/modal-templates/exploration-modify-translations-modal.component'; | ||
|
||
@Component({ | ||
selector: 'oppia-mark-translations-as-needing-update-modal', | ||
|
@@ -30,15 +33,34 @@ export class MarkTranslationsAsNeedingUpdateModalComponent extends ConfirmOrCanc | |
@Input() markNeedsUpdateHandler!: (contentId: string) => void; | ||
@Input() removeHandler!: (contentId: string) => void; | ||
|
||
constructor(private ngbActiveModal: NgbActiveModal) { | ||
modifyTranslationsFeatureFlagIsEnabled: boolean = false; | ||
|
||
constructor( | ||
private ngbActiveModal: NgbActiveModal, | ||
private ngbModal: NgbModal, | ||
private platformFeatureService: PlatformFeatureService | ||
) { | ||
super(ngbActiveModal); | ||
} | ||
|
||
ngOnInit(): void { | ||
this.modifyTranslationsFeatureFlagIsEnabled = | ||
this.platformFeatureService.status.ExplorationEditorCanModifyTranslations.isEnabled; | ||
Comment on lines
+47
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we wrap in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe its needed here and it is pretty standard to have it this way in the codebase, would be needed if the line to the right side of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it may have been missed in other places. Let's stick to what is in the style guide. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel free to ignore wrapping here in case adding parenthesis ends up causing L48 to hit the line character limit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kevintab95 I tried adding the brackets but they got removed automatically by Prettify itself, I assume it is the standard for such lines...? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nikitaevg do you know why this is happening? i.e. prettier seems to work differently on our machines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, no idea TBH. You sure you did save the file? The commit stats say you added 1 line and modified 1 line, but the screenshot suggests much more changes |
||
} | ||
|
||
markNeedsUpdate(): void { | ||
this.markNeedsUpdateHandler(this.contentId); | ||
this.ngbActiveModal.close(); | ||
} | ||
|
||
openModifyTranslationsModal(): void { | ||
const modalRef = this.ngbModal.open(ModifyTranslationsModalComponent, { | ||
backdrop: 'static', | ||
windowClass: 'oppia-modify-translations-modal', | ||
}); | ||
modalRef.componentInstance.contentId = this.contentId; | ||
} | ||
|
||
removeTranslations(): void { | ||
this.removeHandler(this.contentId); | ||
this.ngbActiveModal.close(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,8 @@ import {HttpClientTestingModule} from '@angular/common/http/testing'; | |
import {ExplorationPermissions} from 'domain/exploration/exploration-permissions.model'; | ||
import {WindowRef} from 'services/contextual/window-ref.service'; | ||
import {ExplorationPermissionsBackendApiService} from 'domain/exploration/exploration-permissions-backend-api.service'; | ||
import {EntityTranslationsService} from 'services/entity-translations.services'; | ||
import {EntityTranslation} from 'domain/translation/EntityTranslationObjectFactory'; | ||
|
||
class MockNgbModalRef { | ||
componentInstance = {}; | ||
|
@@ -116,6 +118,7 @@ describe('Exploration editor page component', () => { | |
let registerDeclineTutorialModalEventSpy; | ||
let focusManagerService: FocusManagerService; | ||
let explorationPermissionsBackendApiService: ExplorationPermissionsBackendApiService; | ||
let entityTranslationsService: EntityTranslationsService; | ||
let ngbModal: NgbModal; | ||
let refreshGraphEmitter = new EventEmitter<void>(); | ||
let mockRefreshTranslationTabEventEmitter = new EventEmitter<void>(); | ||
|
@@ -353,6 +356,7 @@ describe('Exploration editor page component', () => { | |
explorationPermissionsBackendApiService = TestBed.inject( | ||
ExplorationPermissionsBackendApiService | ||
); | ||
entityTranslationsService = TestBed.inject(EntityTranslationsService); | ||
|
||
isLocationSetToNonStateEditorTabSpy = spyOn( | ||
rs, | ||
|
@@ -822,7 +826,32 @@ describe('Exploration editor page component', () => { | |
mockInitExplorationPageEmitter | ||
); | ||
explorationData.is_version_of_draft_valid = false; | ||
explorationData.draft_changes = ['data1', 'data2']; | ||
explorationData.draft_changes = [ | ||
{ | ||
cmd: 'edit_translation', | ||
language_code: 'fr', | ||
content_id: 'content0', | ||
translation: { | ||
content_value: '<p>test content one</p>', | ||
content_format: 'html', | ||
needs_update: false, | ||
}, | ||
}, | ||
{ | ||
cmd: 'edit_translation', | ||
language_code: 'fr', | ||
content_id: 'content4', | ||
translation: { | ||
content_value: '<p>test content four</p>', | ||
content_format: 'html', | ||
needs_update: false, | ||
}, | ||
}, | ||
{ | ||
cmd: 'remove_translations', | ||
content_id: 'content0', | ||
}, | ||
]; | ||
|
||
component.ngOnInit(); | ||
}); | ||
|
@@ -901,6 +930,46 @@ describe('Exploration editor page component', () => { | |
discardPeriodicTasks(); | ||
})); | ||
|
||
it('should update entity translations dict with draft changes', fakeAsync(() => { | ||
spyOn(EntityTranslation, 'createFromBackendDict').and.callThrough(); | ||
let entityTranslation = EntityTranslation.createFromBackendDict({ | ||
entity_id: explorationId, | ||
entity_type: 'exploration', | ||
entity_version: explorationData.version, | ||
language_code: 'fr', | ||
translations: { | ||
content4: { | ||
content_value: '<p>test content four</p>', | ||
content_format: 'html', | ||
needs_update: false, | ||
}, | ||
}, | ||
}); | ||
expect( | ||
entityTranslationsService.languageCodeToEntityTranslations | ||
).toEqual({}); | ||
|
||
mockInitExplorationPageEmitter.emit(); | ||
tick(); | ||
|
||
expect(EntityTranslation.createFromBackendDict).toHaveBeenCalledWith({ | ||
entity_id: explorationId, | ||
entity_type: 'exploration', | ||
entity_version: explorationData.version, | ||
language_code: 'fr', | ||
translations: {}, | ||
}); | ||
|
||
expect( | ||
entityTranslationsService.languageCodeToEntityTranslations | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a pre-check for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
).toEqual({ | ||
fr: entityTranslation, | ||
}); | ||
|
||
flush(); | ||
discardPeriodicTasks(); | ||
})); | ||
|
||
it( | ||
'should start editor tutorial when closing welcome exploration' + | ||
' modal', | ||
|
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.
Should there be a pre-check for this? i.e. the value is false before ngOnInit.
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.
Done