-
Notifications
You must be signed in to change notification settings - Fork 43
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
Issue #CO-229 #CO-230 #CO-316 fix: {Reraising PR on release-6.0.0} #303
Changes from 1 commit
8656429
0b37f67
9c4622e
ba0fe42
f180ae8
4774ce6
9220d28
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 |
---|---|---|
@@ -1,13 +1,13 @@ | ||
var cheerio = require("cheerio"); | ||
var cheerioTableparser = require("cheerio-tableparser"); | ||
const sizeOf = require("image-size"); | ||
const { compact } = require("lodash"); | ||
const ProgramServiceHelper = require("../../helpers/programHelper"); | ||
const programServiceHelper = new ProgramServiceHelper(); | ||
const optionLabelling = { | ||
english: {i: 'I', ii: 'II', iii: 'III', iv: 'IV'}, | ||
hindi: {i: 'क', ii: 'ख', iii: 'ग', iv: 'घ'}, | ||
english: {i: 'I', ii: 'II', iii: 'III', iv: 'IV'}, | ||
hindi: {i: 'क', ii: 'ख', iii: 'ग', iv: 'घ'}, | ||
} | ||
const defaultLanguage = 'english' | ||
|
||
var { | ||
docDefinition, | ||
|
@@ -241,6 +241,7 @@ async function getStack(htmlString) { | |
} | ||
|
||
async function renderMCQ(question, questionCounter, marks) { | ||
const printLanguage = (question.medium && question.medium.length) ? question.medium[0].toLowerCase() : defaultLanguage; | ||
const questionOptions = []; | ||
let questionTitle; | ||
for (const [index, qo] of question.editorState.options.entries()) { | ||
|
@@ -274,20 +275,20 @@ async function renderMCQ(question, questionCounter, marks) { | |
typeof questionOptions[0][0] === "object" | ||
) { | ||
if (questionOptions[0][0].text) { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'i')+")"}`, questionOptions[0][0].text[1]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'i')+")"}`, questionOptions[0][0].text[1]]); | ||
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. @vaivk369 suggest the better approach of pushing the question to the array. |
||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
}); | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'i')+")"}`, questionOptions[0][0].image]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'i')+")"}`, questionOptions[0][0].image]); | ||
imageProperties.push({ | ||
width: questionOptions[0][0].width, | ||
height: questionOptions[0][0].height, | ||
}); | ||
} | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'i')+")"}`, questionOptions[0][0]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'i')+")"}`, questionOptions[0][0]]); | ||
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. Why can't you move the entire this logic into different function as pass the index to it
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. This way if there are only 2 option or more than 4 also the code will works. 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. This is done @vinukumar-vs 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. Shall I resolve this conversation, as this is implemented too? |
||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
|
@@ -301,20 +302,20 @@ async function renderMCQ(question, questionCounter, marks) { | |
typeof questionOptions[1][0] === "object" | ||
) { | ||
if (questionOptions[1][0].text) { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'ii')+")"}`, questionOptions[1][0].text[1]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'ii')+")"}`, questionOptions[1][0].text[1]]); | ||
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. Why should I use the same key "II" for all the languages. 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. Cause there must be default options labeling in case of undefined language. 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. Shall I resolve this conversation? |
||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
}); | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'ii')+")"}`, questionOptions[1][0].image]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'ii')+")"}`, questionOptions[1][0].image]); | ||
imageProperties.push({ | ||
width: questionOptions[1][0].width, | ||
height: questionOptions[1][0].height, | ||
}); | ||
} | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'ii')+")"}`, questionOptions[1][0]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'ii')+")"}`, questionOptions[1][0]]); | ||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
|
@@ -328,20 +329,20 @@ async function renderMCQ(question, questionCounter, marks) { | |
typeof questionOptions[2][0] === "object" | ||
) { | ||
if (questionOptions[2][0].text) { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iii')+")"}`, questionOptions[2][0].text[1]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iii')+")"}`, questionOptions[2][0].text[1]]); | ||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
}); | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iii')+")"}`, questionOptions[2][0].image]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iii')+")"}`, questionOptions[2][0].image]); | ||
imageProperties.push({ | ||
width: questionOptions[2][0].width, | ||
height: questionOptions[2][0].height, | ||
}); | ||
} | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iii')+")"}`, questionOptions[2][0]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iii')+")"}`, questionOptions[2][0]]); | ||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
|
@@ -355,20 +356,20 @@ async function renderMCQ(question, questionCounter, marks) { | |
typeof questionOptions[3][0] === "object" | ||
) { | ||
if (questionOptions[3][0].text) { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iv')+")"}`, questionOptions[3][0].text[1]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iv')+")"}`, questionOptions[3][0].text[1]]); | ||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
}); | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iv')+")"}`, questionOptions[3][0].image]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iv')+")"}`, questionOptions[3][0].image]); | ||
imageProperties.push({ | ||
width: questionOptions[3][0].width, | ||
height: questionOptions[3][0].height, | ||
}); | ||
} | ||
} else { | ||
questionOpt.push([`${getLanguageKey(question.medium[0].toLowerCase(), 'iv')+")"}`, questionOptions[3][0]]); | ||
questionOpt.push([`${getLanguageKey(printLanguage, 'iv')+")"}`, questionOptions[3][0]]); | ||
imageProperties.push({ | ||
width: 0, | ||
height: 0, | ||
|
@@ -503,9 +504,9 @@ async function renderMTF(question, questionCounter, marks, Type) { | |
return mtfData; | ||
} | ||
function getLanguageKey(lang, key) { | ||
return optionLabelling[lang] && optionLabelling[lang][key] ? | ||
optionLabelling[lang][key] : | ||
optionLabelling['english'][key] | ||
return optionLabelling[lang] && optionLabelling[lang][key] ? | ||
optionLabelling[lang][key] : | ||
optionLabelling[defaultLanguage][key] | ||
} | ||
|
||
module.exports = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,8 +26,7 @@ const basicDetails={ | |
} | ||
|
||
function create(data, paperData) { | ||
const language = paperData?.language?.toLowerCase(); | ||
let basicPaperDetials = basicDetails.english; | ||
const language = (paperData && paperData.language) ? paperData.language.toLowerCase(): defaultLanguage; | ||
|
||
const doc = new Document({ | ||
sections: [ | ||
|
@@ -235,9 +234,9 @@ function create(data, paperData) { | |
count++; | ||
}); | ||
arr.push( | ||
new Paragraph({ | ||
children: [], // Just newline without text | ||
}) | ||
// new Paragraph({ | ||
// children: [], // Just newline without text | ||
// }) | ||
); | ||
arr.push(optionsTabel(testimage)); | ||
arr.push( | ||
|
@@ -720,8 +719,8 @@ function displayNumber(data) { | |
type: WidthType.DXA, | ||
}, | ||
margins: { | ||
top: convertInchesToTwip(0.0693701), | ||
bottom: convertInchesToTwip(0.0693701), | ||
top: convertInchesToTwip(0.0093701), | ||
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. This is not the right way of fixing the margins. It should be common across all sides. Any specific reason why the margins are different for the document. 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. While printing paper having MCQ questions, the line height between line 1 of options and line 2 of options was a bit higher. It was done to reduce the line height space to consume less physical paper. 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. Not convinced.. Can you share the snapshot of the UI. 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. |
||
bottom: convertInchesToTwip(0.0093701), | ||
left: convertInchesToTwip(0.3493701), | ||
right: convertInchesToTwip(0.0693701), | ||
}, | ||
|
@@ -1002,11 +1001,11 @@ function optionsTabel(testimage) { | |
} | ||
|
||
function getLanguageKey(lang, key) { | ||
return basicDetails[lang] && basicDetails[lang][key] ? | ||
basicDetails[lang][key] : | ||
return basicDetails[lang] && basicDetails[lang][key] ? | ||
basicDetails[lang][key] : | ||
basicDetails[defaultLanguage][key] | ||
} | ||
|
||
module.exports = { | ||
create, | ||
}; | ||
}; |
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.
Why are the using the keys as i, ii, iii, iv, why can't it be just an array of strings?
optionLabelling = {
"en": ["I", "II", "III" ......],
"hi": ["क", "ख", "ग" ......]
}
This should be configuration driven. should not hardcode here. Later if we want another language, it should be config change. Code should work.
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.
Yes will be implementing this.
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.
Shall I resolve this conversation, as this one is implemented?