Skip to content

Commit

Permalink
Fix issue oppia#21081: changed drop-down to heading in the question e…
Browse files Browse the repository at this point in the history
…ditor. (oppia#21171)

* Update UI with CSS changes and conditional rendering

* Subscribe to topicEditorStateService and utilize its methods

* added _questionEditorOpened state, _questionEditorOpenedEventEmitter EventEmitter & new get methods

* replicated the working for new question editor

* add tests for drop-down toggle behaviour

* fix failing e2e test
  • Loading branch information
ujjwalpathaak authored Oct 27, 2024
1 parent 60b3dab commit 05ded5f
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="question-editor-container">
<div *ngIf="editorIsOpen" class="question-editor-parent">
<mat-card class="oppia-page-card oppia-mobile-collapsible-card">
<mat-card class="oppia-page-card oppia-question-editor-page-card oppia-mobile-collapsible-card">
<div class="difficulty-card-header oppia-mobile-collapsible-card-header" (click)="toggleDifficultyCard()">
<h3>Difficulty</h3>
<i class="fa fa-caret-down"
Expand Down Expand Up @@ -329,6 +329,10 @@ <h3>Create a new question</h3>
.list-group {
gap: 10px;
}
.oppia-question-editor-page-card {
margin-bottom: 0;
margin-top: 0;
}
@media screen and (max-width: 768px) {
.question-editor-container {
margin-top: 30px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {ImageLocalStorageService} from 'services/image-local-storage.service';
import {QuestionsListService} from 'services/questions-list.service';
import {QuestionValidationService} from 'services/question-validation.service';
import {SkillEditorRoutingService} from 'pages/skill-editor-page/services/skill-editor-routing.service';
import {TopicEditorStateService} from 'pages/topic-editor-page/services/topic-editor-state.service';
import {UtilsService} from 'services/utils.service';
import {LoggerService} from 'services/contextual/logger.service';
import {WindowDimensionsService} from 'services/contextual/window-dimensions.service';
Expand Down Expand Up @@ -131,7 +132,8 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
private skillEditorRoutingService: SkillEditorRoutingService,
private utilsService: UtilsService,
private windowDimensionsService: WindowDimensionsService,
private windowRef: WindowRef
private windowRef: WindowRef,
private topicEditorStateService: TopicEditorStateService
) {}

createQuestion(): void {
Expand Down Expand Up @@ -168,6 +170,10 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
this.questionStateData = this.question.getStateData();
this.questionIsBeingUpdated = false;
this.newQuestionIsBeingCreated = true;
this.topicEditorStateService.toggleQuestionEditor(
true,
this.newQuestionIsBeingCreated
);
this.editorIsOpen = true;

this.skillLinkageModificationsArray = [];
Expand Down Expand Up @@ -282,6 +288,7 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
openQuestionEditor(): void {
this.questionUndoRedoService.clearChanges();
this.editorIsOpen = true;
this.topicEditorStateService.toggleQuestionEditor(true);
this.imageLocalStorageService.flushStoredImagesData();
if (this.newQuestionIsBeingCreated) {
this.contextService.setImageSaveDestinationToLocalStorage();
Expand Down Expand Up @@ -622,6 +629,7 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
() => {
this.contextService.resetImageSaveDestination();
this.editorIsOpen = false;
this.topicEditorStateService.toggleQuestionEditor(false);
this.windowRef.nativeWindow.location.hash = null;
this.skillEditorRoutingService.questionIsBeingCreated = false;
},
Expand Down Expand Up @@ -685,12 +693,14 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
this.contextService.resetImageSaveDestination();
this.saveAndPublishQuestion(commitMessage);
}
this.topicEditorStateService.toggleQuestionEditor(false);
},
() => {
this.questionIsBeingSaved = false;
}
);
} else {
this.topicEditorStateService.toggleQuestionEditor(false);
this.contextService.resetImageSaveDestination();
this.saveAndPublishQuestion(null);
this.skillEditorRoutingService.creatingNewQuestion(false);
Expand Down Expand Up @@ -750,6 +760,9 @@ export class QuestionsListComponent implements OnInit, OnDestroy {
this.changeDetectorRef.detectChanges();
})
);
this.directiveSubscriptions.add(
this.topicEditorStateService.onQuestionEditorOpened.subscribe()
);

this.showDifficultyChoices = false;
this.difficultyCardIsShown = !this.windowDimensionsService.isWindowNarrow();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
<div class="select-skill-dropdown-container">
<div class="question-editor-header-container" *ngIf="questionEditorOpened">
<mat-card class="oppia-page-card oppia-mobile-collapsible-card question-editor-header">
<h3>{{ getEditorAction() }} question for skill: {{ selectedSkillName }}</h3>
</mat-card>
</div>
<div class="select-skill-dropdown-container" *ngIf="!questionEditorOpened">
<mat-form-field class="form-field e2e-test-select-skill-dropdown"
appearance="fill"
*ngIf="allSkillSummaries.length > 0">
Expand Down Expand Up @@ -42,6 +47,7 @@
margin: 0 auto;
width: 70%;
}
.question-editor-header-container,
.select-skill-dropdown-container {
margin: 0 auto;
width: 70%;
Expand All @@ -52,4 +58,12 @@
.question-list-container {
margin-top: -30px;
}
@media screen and (max-width: 768px) {
.question-editor-header-container {
width: 100%;
}
.question-editor-header {
margin-bottom: 45px;
}
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
} from 'domain/topics_and_skills_dashboard/topics-and-skills-dashboard-backend-api.service';
import {HttpClientTestingModule} from '@angular/common/http/testing';
import {TopicQuestionsTabComponent} from './topic-questions-tab.component';
import {QuestionsListComponent} from '../../../components/question-directives/questions-list/questions-list.component';
import {TopicEditorStateService} from '../services/topic-editor-state.service';
import {Topic} from 'domain/topic/topic-object.model';

Expand Down Expand Up @@ -101,6 +102,7 @@ class MockTopicsAndSkillsDashboardBackendApiService {

describe('Topic questions tab', () => {
let component: TopicQuestionsTabComponent;
let questionsListComponent: QuestionsListComponent;
let fixture: ComponentFixture<TopicQuestionsTabComponent>;
let topicEditorStateService: TopicEditorStateService;
let focusManagerService: FocusManagerService;
Expand All @@ -113,7 +115,7 @@ describe('Topic questions tab', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [HttpClientTestingModule],
declarations: [TopicQuestionsTabComponent],
declarations: [TopicQuestionsTabComponent, QuestionsListComponent],
providers: [
TopicsAndSkillsDashboardBackendApiService,
{
Expand All @@ -128,6 +130,10 @@ describe('Topic questions tab', () => {
beforeEach(() => {
fixture = TestBed.createComponent(TopicQuestionsTabComponent);
component = fixture.componentInstance;
const questionsListFixture = TestBed.createComponent(
QuestionsListComponent
);
questionsListComponent = questionsListFixture.componentInstance;
qls = TestBed.inject(QuestionsListService);
focusManagerService = TestBed.inject(FocusManagerService);
topicEditorStateService = TestBed.inject(TopicEditorStateService);
Expand Down Expand Up @@ -268,4 +274,29 @@ describe('Topic questions tab', () => {
expect(component.topicRights).toEqual(topicRights);
expect(component.topic).toBe(topic);
});

describe('should change drop-down to a heading when the questionEditor is opened', () => {
it('should return "Creating new" when creating a new question', () => {
spyOn(questionsListComponent, 'createQuestion').and.callThrough();
spyOn(topicEditorStateService, 'toggleQuestionEditor').and.callThrough();

questionsListComponent.createQuestion();
expect(topicEditorStateService.toggleQuestionEditor).toHaveBeenCalledWith(
true,
true
);
expect(component.getEditorAction()).toBe('Creating new');
});

it('should return "Editing" when editing a question', () => {
spyOn(questionsListComponent, 'openQuestionEditor').and.callThrough();
spyOn(topicEditorStateService, 'toggleQuestionEditor').and.callThrough();

questionsListComponent.openQuestionEditor();
expect(topicEditorStateService.toggleQuestionEditor).toHaveBeenCalledWith(
true
);
expect(component.getEditorAction()).toBe('Editing');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ export class TopicQuestionsTabComponent
skillIdToRubricsObject!: object;
allSkillSummaries!: ShortSkillSummary[];
canEditQuestion!: boolean;
questionEditorOpened: boolean = false;
newQuestionEditor: boolean = false;
selectedSkillName!: string;
selectedSkillId!: string;
getSkillsCategorizedByTopics!: CategorizedSkills;
getUntriagedSkillSummaries!: SkillSummary[];
Expand Down Expand Up @@ -81,6 +84,17 @@ export class TopicQuestionsTabComponent
this.getUntriagedSkillSummaries = response.untriagedSkillSummaries;
});
this.canEditQuestion = this.topicRights.canEditTopic();
this.questionEditorOpened =
this.topicEditorStateService.isQuestionEditorOpened();
this.newQuestionEditor = this.topicEditorStateService.isNewQuestionEditor();
this.selectedSkillName = this.topicEditorStateService.getSelectedSkillName(
this.selectedSkillId,
this.allSkillSummaries
);
}

getEditorAction(): string {
return this.newQuestionEditor ? 'Creating new' : 'Editing';
}

reinitializeQuestionsList(skillId: string): void {
Expand Down Expand Up @@ -108,11 +122,17 @@ export class TopicQuestionsTabComponent
this._initTab()
)
);
this.directiveSubscriptions.add(
this.topicEditorStateService.onQuestionEditorOpened.subscribe(() =>
this._initTab()
)
);
this._initTab();
}

ngOnDestroy(): void {
this.directiveSubscriptions.unsubscribe();
this.topicEditorStateService.toggleQuestionEditor(false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
} from 'domain/editor/undo_redo/change.model';
import {LoaderService} from 'services/loader.service';
import {SubtopicPageContents} from 'domain/topic/subtopic-page-contents.model';
import {ShortSkillSummary} from 'domain/skill/short-skill-summary.model';

interface GroupedSkillSummaryDict {
current: SkillSummaryBackendDict[];
Expand Down Expand Up @@ -87,6 +88,8 @@ export class TopicEditorStateService {
private _classroomUrlFragment: string | null = null;
private _curriculumAdminUsernames: string[] = [];
private _classroomName: string | null = null;
private _questionEditorOpened: boolean = false;
private _newQuestionEditor: boolean = false;
private _storySummariesInitializedEventEmitter: EventEmitter<void> =
new EventEmitter();

Expand All @@ -99,6 +102,9 @@ export class TopicEditorStateService {
private _topicReinitializedEventEmitter: EventEmitter<void> =
new EventEmitter();

private _questionEditorOpenedEventEmitter: EventEmitter<boolean> =
new EventEmitter<boolean>();

constructor(
private alertsService: AlertsService,
private editableStoryBackendApiService: EditableStoryBackendApiService,
Expand Down Expand Up @@ -317,6 +323,14 @@ export class TopicEditorStateService {
);
}

isQuestionEditorOpened(): boolean {
return this._questionEditorOpened;
}

isNewQuestionEditor(): boolean {
return this._newQuestionEditor;
}

getGroupedSkillSummaries(): object {
return cloneDeep(this._groupedSkillSummaries);
}
Expand Down Expand Up @@ -464,6 +478,28 @@ export class TopicEditorStateService {
}
}

/**
* When the editor is opened (editorOpened is true),
* assign the value of newQuestion to _newQuestionEditor,
* and set _questionEditorOpened to true.
* When the editor is closed (editorOpened is false),
* both flags are set to false.
*/
toggleQuestionEditor(
editorOpened: boolean,
newQuestion: boolean = false
): void {
if (editorOpened) {
this._newQuestionEditor = newQuestion;
this._questionEditorOpened = true;
} else {
this._newQuestionEditor = false;
this._questionEditorOpened = false;
}

this._questionEditorOpenedEventEmitter.emit();
}

deleteSubtopicPage(topicId: string, subtopicId: number): void {
let subtopicPageId = this._getSubtopicPageId(topicId, subtopicId);
let index = this._getSubtopicPageIndex(subtopicPageId);
Expand Down Expand Up @@ -605,6 +641,10 @@ export class TopicEditorStateService {
return this._topicReinitializedEventEmitter;
}

get onQuestionEditorOpened(): EventEmitter<boolean> {
return this._questionEditorOpenedEventEmitter;
}

/**
* Returns the classroom url fragment if the topic is assigned to any
* classroom, else null.
Expand All @@ -625,6 +665,22 @@ export class TopicEditorStateService {
return this._curriculumAdminUsernames;
}

/**
* Retrieves the name of the skill associated with the given skill ID,
* typically the skill whose question is currently being edited.
* The skill description will always be present in allSkillSummaries,
* but to ensure robustness, the skill ID will be displayed in case of a failure.
*/
getSelectedSkillName(
skillId: string,
allSkillSummaries: ShortSkillSummary[]
): string {
return (
allSkillSummaries?.find(skill => skill.id === skillId)?.description ||
skillId
);
}

/**
* Attempts to set the boolean variable _topicWithNameExists based
* on the value returned by doesTopicWithNameExistAsync and
Expand Down

0 comments on commit 05ded5f

Please sign in to comment.