From eb04f36892c4ee878c28a54900b8c1d7e2875f7e Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Mon, 19 Feb 2024 20:56:58 +0000 Subject: [PATCH 1/5] Remove jQuery from repo wiki creation page - Switched to plain JavaScript - Tested the wiki creation form functionality and it works as before Signed-off-by: Yarden Shoham --- .../js/features/comp/ComboMarkdownEditor.js | 17 +++--- web_src/js/features/repo-wiki.js | 56 ++++++++++--------- web_src/js/utils/dom.js | 17 ++++++ 3 files changed, 54 insertions(+), 36 deletions(-) diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index d486c5830acc8..8980890593072 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -2,7 +2,7 @@ import '@github/markdown-toolbar-element'; import '@github/text-expander-element'; import $ from 'jquery'; import {attachTribute} from '../tribute.js'; -import {hideElem, showElem, autosize} from '../../utils/dom.js'; +import {hideElem, showElem, autosize, isVisible} from '../../utils/dom.js'; import {initEasyMDEImagePaste, initTextareaImagePaste} from './ImagePaste.js'; import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js'; import {renderPreviewPanelContent} from '../repo-editor.js'; @@ -14,19 +14,18 @@ let elementIdCounter = 0; /** * validate if the given textarea is non-empty. - * @param {jQuery} $textarea + * @param {HTMLElement} textarea - The textarea element to be validated. * @returns {boolean} returns true if validation succeeded. */ -export function validateTextareaNonEmpty($textarea) { +export function validateTextareaNonEmpty(textarea) { // When using EasyMDE, the original edit area HTML element is hidden, breaking HTML5 input validation. // The workaround (https://github.com/sparksuite/simplemde-markdown-editor/issues/324) doesn't work with contenteditable, so we just show an alert. - if (!$textarea.val()) { - if ($textarea.is(':visible')) { - $textarea.prop('required', true); - const $form = $textarea.parents('form'); - $form[0]?.reportValidity(); + if (!textarea.value) { + if (isVisible(textarea)) { + textarea.setAttribute('required', 'true'); + const form = textarea.closest('form'); + form?.reportValidity(); } else { - // The alert won't hurt users too much, because we are dropping the EasyMDE and the check only occurs in a few places. showErrorToast('Require non-empty content'); } return false; diff --git a/web_src/js/features/repo-wiki.js b/web_src/js/features/repo-wiki.js index 58036fde3702b..d51bf35c810b4 100644 --- a/web_src/js/features/repo-wiki.js +++ b/web_src/js/features/repo-wiki.js @@ -1,50 +1,51 @@ -import $ from 'jquery'; import {initMarkupContent} from '../markup/content.js'; import {validateTextareaNonEmpty, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js'; import {fomanticMobileScreen} from '../modules/fomantic.js'; - -const {csrfToken} = window.config; +import {POST} from '../modules/fetch.js'; async function initRepoWikiFormEditor() { - const $editArea = $('.repository.wiki .combo-markdown-editor textarea'); - if (!$editArea.length) return; + const editArea = document.querySelector('.repository.wiki .combo-markdown-editor textarea'); + if (!editArea) return; - const $form = $('.repository.wiki.new .ui.form'); - const $editorContainer = $form.find('.combo-markdown-editor'); + const form = document.querySelector('.repository.wiki.new .ui.form'); + const editorContainer = form.querySelector('.combo-markdown-editor'); let editor; let renderRequesting = false; let lastContent; - const renderEasyMDEPreview = function () { + const renderEasyMDEPreview = async function () { if (renderRequesting) return; - const $previewFull = $editorContainer.find('.EasyMDEContainer .editor-preview-active'); - const $previewSide = $editorContainer.find('.EasyMDEContainer .editor-preview-active-side'); - const $previewTarget = $previewSide.length ? $previewSide : $previewFull; - const newContent = $editArea.val(); - if (editor && $previewTarget.length && lastContent !== newContent) { + const previewFull = editorContainer.querySelector('.EasyMDEContainer .editor-preview-active'); + const previewSide = editorContainer.querySelector('.EasyMDEContainer .editor-preview-active-side'); + const previewTarget = previewSide || previewFull; + const newContent = editArea.value; + if (editor && previewTarget && lastContent !== newContent) { renderRequesting = true; - $.post(editor.previewUrl, { - _csrf: csrfToken, - mode: editor.previewMode, - context: editor.previewContext, - text: newContent, - wiki: editor.previewWiki, - }).done((data) => { + const formData = new FormData(); + formData.append('mode', editor.previewMode); + formData.append('context', editor.previewContext); + formData.append('text', newContent); + formData.append('wiki', editor.previewWiki); + try { + const response = await POST(editor.previewUrl, {data: formData}); + const data = await response.text(); lastContent = newContent; - $previewTarget.html(`
${data}
`); + previewTarget.innerHTML = `
${data}
`; initMarkupContent(); - }).always(() => { + } catch (error) { + console.error('Error rendering preview:', error); + } finally { renderRequesting = false; setTimeout(renderEasyMDEPreview, 1000); - }); + } } else { setTimeout(renderEasyMDEPreview, 1000); } }; renderEasyMDEPreview(); - editor = await initComboMarkdownEditor($editorContainer, { + editor = await initComboMarkdownEditor(editorContainer, { useScene: 'wiki', // EasyMDE has some problems of height definition, it has inline style height 300px by default, so we also use inline styles to override it. // And another benefit is that we only need to write the style once for both editors. @@ -64,9 +65,10 @@ async function initRepoWikiFormEditor() { }, }); - $form.on('submit', () => { - if (!validateTextareaNonEmpty($editArea)) { - return false; + form.addEventListener('submit', (e) => { + if (!validateTextareaNonEmpty(editArea)) { + e.preventDefault(); + e.stopPropagation(); } }); } diff --git a/web_src/js/utils/dom.js b/web_src/js/utils/dom.js index fb6b751140307..09f3f2041fec3 100644 --- a/web_src/js/utils/dom.js +++ b/web_src/js/utils/dom.js @@ -227,3 +227,20 @@ export function initSubmitEventPolyfill() { document.body.addEventListener('click', submitEventPolyfillListener); document.body.addEventListener('focus', submitEventPolyfillListener); } + +/** + * Check if an element is visible. + * Note: This function doesn't account for all possible visibility scenarios. + * @param {HTMLElement} element The element to check. + * @returns {boolean} True if the element is visible. + */ +export function isVisible(element) { + if (!element) return false; + + const style = window.getComputedStyle(element); + return style.width !== '0' && + style.height !== '0' && + style.opacity !== '0' && + style.display !== 'none' && + style.visibility !== 'hidden'; +} From f695a012f2169b7fd2843894e19a35592eea913c Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Mon, 19 Feb 2024 21:30:16 +0000 Subject: [PATCH 2/5] Use jQuery visibility logic --- web_src/js/utils/dom.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/web_src/js/utils/dom.js b/web_src/js/utils/dom.js index 09f3f2041fec3..f21d4bfcfc7b8 100644 --- a/web_src/js/utils/dom.js +++ b/web_src/js/utils/dom.js @@ -237,10 +237,5 @@ export function initSubmitEventPolyfill() { export function isVisible(element) { if (!element) return false; - const style = window.getComputedStyle(element); - return style.width !== '0' && - style.height !== '0' && - style.opacity !== '0' && - style.display !== 'none' && - style.visibility !== 'hidden'; + return Boolean(element.offsetWidth || element.offsetHeight || element.getClientRects().length); } From 16ccfcddbf143c3bfdef51836ede80f115a27f8b Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 19 Feb 2024 22:54:02 +0100 Subject: [PATCH 3/5] Update web_src/js/utils/dom.js --- web_src/js/utils/dom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/utils/dom.js b/web_src/js/utils/dom.js index f21d4bfcfc7b8..dc168e8954a2d 100644 --- a/web_src/js/utils/dom.js +++ b/web_src/js/utils/dom.js @@ -229,7 +229,7 @@ export function initSubmitEventPolyfill() { } /** - * Check if an element is visible. + * Check if an element is visible, equivalent to jQuery's `:visible` pseudo. * Note: This function doesn't account for all possible visibility scenarios. * @param {HTMLElement} element The element to check. * @returns {boolean} True if the element is visible. From f658668951a7934f5e362b0b0d9526b67ecf17b2 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Tue, 20 Feb 2024 09:11:40 +0000 Subject: [PATCH 4/5] Apply code review suggestions --- web_src/js/features/comp/ComboMarkdownEditor.js | 7 ++++--- web_src/js/utils/dom.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index 8980890593072..d209f11ab2fab 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -2,7 +2,7 @@ import '@github/markdown-toolbar-element'; import '@github/text-expander-element'; import $ from 'jquery'; import {attachTribute} from '../tribute.js'; -import {hideElem, showElem, autosize, isVisible} from '../../utils/dom.js'; +import {hideElem, showElem, autosize, isElemVisible} from '../../utils/dom.js'; import {initEasyMDEImagePaste, initTextareaImagePaste} from './ImagePaste.js'; import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js'; import {renderPreviewPanelContent} from '../repo-editor.js'; @@ -21,11 +21,12 @@ export function validateTextareaNonEmpty(textarea) { // When using EasyMDE, the original edit area HTML element is hidden, breaking HTML5 input validation. // The workaround (https://github.com/sparksuite/simplemde-markdown-editor/issues/324) doesn't work with contenteditable, so we just show an alert. if (!textarea.value) { - if (isVisible(textarea)) { - textarea.setAttribute('required', 'true'); + if (isElemVisible(textarea)) { + textarea.required = true; const form = textarea.closest('form'); form?.reportValidity(); } else { + // The alert won't hurt users too much, because we are dropping the EasyMDE and the check only occurs in a few places. showErrorToast('Require non-empty content'); } return false; diff --git a/web_src/js/utils/dom.js b/web_src/js/utils/dom.js index dc168e8954a2d..ca24650f76ac3 100644 --- a/web_src/js/utils/dom.js +++ b/web_src/js/utils/dom.js @@ -234,7 +234,7 @@ export function initSubmitEventPolyfill() { * @param {HTMLElement} element The element to check. * @returns {boolean} True if the element is visible. */ -export function isVisible(element) { +export function isElemVisible(element) { if (!element) return false; return Boolean(element.offsetWidth || element.offsetHeight || element.getClientRects().length); From 8d74e012acea66e18948f899b85b5fe138b6b494 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Tue, 20 Feb 2024 09:27:19 +0000 Subject: [PATCH 5/5] Fix all callers --- web_src/js/features/repo-diff.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 6d6f382613d38..5c73bf4bbc08f 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -47,8 +47,8 @@ function initRepoDiffConversationForm() { e.preventDefault(); const $form = $(e.target); - const $textArea = $form.find('textarea'); - if (!validateTextareaNonEmpty($textArea)) { + const textArea = e.target.querySelector('textarea'); + if (!validateTextareaNonEmpty(textArea)) { return; }