Skip to content

Commit f658668

Browse files
committed
Apply code review suggestions
1 parent 16ccfcd commit f658668

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

web_src/js/features/comp/ComboMarkdownEditor.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import '@github/markdown-toolbar-element';
22
import '@github/text-expander-element';
33
import $ from 'jquery';
44
import {attachTribute} from '../tribute.js';
5-
import {hideElem, showElem, autosize, isVisible} from '../../utils/dom.js';
5+
import {hideElem, showElem, autosize, isElemVisible} from '../../utils/dom.js';
66
import {initEasyMDEImagePaste, initTextareaImagePaste} from './ImagePaste.js';
77
import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js';
88
import {renderPreviewPanelContent} from '../repo-editor.js';
@@ -21,11 +21,12 @@ export function validateTextareaNonEmpty(textarea) {
2121
// When using EasyMDE, the original edit area HTML element is hidden, breaking HTML5 input validation.
2222
// The workaround (https://github.com/sparksuite/simplemde-markdown-editor/issues/324) doesn't work with contenteditable, so we just show an alert.
2323
if (!textarea.value) {
24-
if (isVisible(textarea)) {
25-
textarea.setAttribute('required', 'true');
24+
if (isElemVisible(textarea)) {
25+
textarea.required = true;
2626
const form = textarea.closest('form');
2727
form?.reportValidity();
2828
} else {
29+
// The alert won't hurt users too much, because we are dropping the EasyMDE and the check only occurs in a few places.
2930
showErrorToast('Require non-empty content');
3031
}
3132
return false;

web_src/js/utils/dom.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ export function initSubmitEventPolyfill() {
234234
* @param {HTMLElement} element The element to check.
235235
* @returns {boolean} True if the element is visible.
236236
*/
237-
export function isVisible(element) {
237+
export function isElemVisible(element) {
238238
if (!element) return false;
239239

240240
return Boolean(element.offsetWidth || element.offsetHeight || element.getClientRects().length);

0 commit comments

Comments
 (0)