-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix/AUT-2673/inline-figure-img #528
Changes from 5 commits
d57654b
e686533
f6b30ec
73af5aa
86fb44a
6bf18d9
4967e53
9119c90
237ed65
fc1189f
7cf054d
a39aca8
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 |
---|---|---|
|
@@ -148,3 +148,7 @@ | |
.bg-error { | ||
background-color: $errorBgColor; | ||
} | ||
|
||
textarea + .feedback-info { | ||
padding: 5px; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,12 +97,14 @@ const formCallbacks = ({ widget, formElement, mediaEditor, togglePlaceholder }) | |
const initForm = ({ widget, formElement, formTpl, mediaEditor, togglePlaceholder }) => { | ||
const imageElem = getImageElement(widget); | ||
const figcaptionElem = getCaptionElement(widget); | ||
const showFigure = widget.element.attr('showFigure'); | ||
widget.$form.html( | ||
formTpl({ | ||
baseUrl: widget.options.baseUrl || '', | ||
src: imageElem.attr('src'), | ||
alt: imageElem.attr('alt'), | ||
figcaption: figcaptionElem ? figcaptionElem.body() : '' | ||
figcaption: figcaptionElem ? figcaptionElem.body() : '', | ||
showFigure: showFigure | ||
}) | ||
); | ||
|
||
|
@@ -125,7 +127,7 @@ const initForm = ({ widget, formElement, formTpl, mediaEditor, togglePlaceholder | |
); | ||
}; | ||
|
||
export default function (stateFactory, ActiveState, formTpl, formElement, inlineHelper) { | ||
export default function (stateFactory, ActiveState, formTpl, formElement, inlineHelper, htmlEditor) { | ||
/** | ||
* media Editor instance if has been initialized | ||
* @type {null} | ||
|
@@ -145,6 +147,7 @@ export default function (stateFactory, ActiveState, formTpl, formElement, inline | |
textareaObserver.unobserve(texareaHTMLElem); | ||
} | ||
this.widget.$form.empty(); | ||
this.destroyEditor(); | ||
} | ||
); | ||
|
||
|
@@ -170,5 +173,14 @@ export default function (stateFactory, ActiveState, formTpl, formElement, inline | |
} | ||
}; | ||
|
||
ImgStateActive.prototype.destroyEditor = function () { | ||
const widget = this.widget; | ||
const $editableContainer = widget.$original.parents('[data-qti-class="_container"]'); | ||
htmlEditor.buildEditor($editableContainer, {}); | ||
_.defer(() => { | ||
htmlEditor.destroyEditor($editableContainer); | ||
}); | ||
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. question: Why building then destroying the editor immediately? 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. The CKEditor reads the Figure HTML tag, and if it is inside a block, it moves out of it. |
||
}; | ||
|
||
return ImgStateActive; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -43,12 +43,17 @@ export const positionFloat = function positionFloat(widget, position) { | |||||
// Update DOM | ||||||
widget.$container.addClass(className); | ||||||
// Update model | ||||||
const prevClassName = widget.element.attr('class'); | ||||||
if (className) { | ||||||
widget.element.attr('class', className); | ||||||
} else { | ||||||
widget.element.removeAttr('class'); | ||||||
} | ||||||
|
||||||
|
||||||
if ((prevClassName || className.length) && prevClassName !== className) { | ||||||
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. question: Why test the value only for one variable and the length only for the other variable? Both are string, so it could be tested by value for both... suggestion:
Suggested change
|
||||||
// Re-build Figure widget to toggle between inline/block | ||||||
widget.refresh(widget.$container); | ||||||
} | ||||||
widget.$original.trigger('contentChange.qti-widget'); | ||||||
}; | ||||||
|
||||||
|
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.
quibble: The signatures becomes heavier and heavier. Maybe it is the right time to think about simplifying it. If the API is not used in too many places, it could be simplified with a config object, allowing to addition of more entries without breaking the signature.
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.
Good point!