From 90571001823bf5aac7c2e377f76de6bd632c1c6b Mon Sep 17 00:00:00 2001 From: Nirmal Kumar R Date: Sat, 23 Nov 2024 15:00:18 +0000 Subject: [PATCH] fix: Preview picture not visible on Markdown file (#5781) Extend API MarkupOptions to contain branch path. The `api.MarkupOptions{}` to have `BranchPath` which contains the current branch. The `RenderMarkup` function utilizes a struct since there are too many variables passed as arguments and that is not a good sign for readability. And `repo-editor.js` will contain a new form data which is `branch-path` which will then be utilized by `edit.tmpl` as `data-branch-path`. Closes: #4510 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [ ] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5781 Reviewed-by: Earl Warren Co-authored-by: Nirmal Kumar R Co-committed-by: Nirmal Kumar R --- modules/structs/miscellaneous.go | 4 +++ routers/api/v1/misc/markup.go | 20 +++++++++++++-- routers/common/markup.go | 36 ++++++++++++++++----------- routers/web/misc/markup.go | 12 ++++++++- routers/web/repo/editor.go | 1 + templates/repo/editor/edit.tmpl | 2 +- templates/swagger/v1_json.tmpl | 4 +++ tests/e2e/markdown-editor.test.e2e.ts | 31 +++++++++++++++++++++++ web_src/js/features/repo-editor.js | 1 + 9 files changed, 92 insertions(+), 19 deletions(-) diff --git a/modules/structs/miscellaneous.go b/modules/structs/miscellaneous.go index bff10f95b7..7866cb5fc0 100644 --- a/modules/structs/miscellaneous.go +++ b/modules/structs/miscellaneous.go @@ -37,6 +37,10 @@ type MarkupOption struct { // // in: body FilePath string + // The current branch path where the form gets posted + // + // in: body + BranchPath string } // MarkupRender is a rendered markup document diff --git a/routers/api/v1/misc/markup.go b/routers/api/v1/misc/markup.go index 9699c79368..155ad15712 100644 --- a/routers/api/v1/misc/markup.go +++ b/routers/api/v1/misc/markup.go @@ -41,7 +41,16 @@ func Markup(ctx *context.APIContext) { return } - common.RenderMarkup(ctx.Base, ctx.Repo, form.Mode, form.Text, form.Context, form.FilePath, form.Wiki) + re := common.Renderer{ + Mode: form.Mode, + Text: form.Text, + URLPrefix: form.Context, + FilePath: form.FilePath, + BranchPath: form.BranchPath, + IsWiki: form.Wiki, + } + + re.RenderMarkup(ctx.Base, ctx.Repo) } // Markdown render markdown document to HTML @@ -76,7 +85,14 @@ func Markdown(ctx *context.APIContext) { mode = form.Mode } - common.RenderMarkup(ctx.Base, ctx.Repo, mode, form.Text, form.Context, "", form.Wiki) + re := common.Renderer{ + Mode: mode, + Text: form.Text, + URLPrefix: form.Context, + IsWiki: form.Wiki, + } + + re.RenderMarkup(ctx.Base, ctx.Repo) } // MarkdownRaw render raw markdown HTML diff --git a/routers/common/markup.go b/routers/common/markup.go index 2d5638ef61..ce3a8acdb0 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -18,26 +18,31 @@ import ( "mvdan.cc/xurls/v2" ) +type Renderer struct { + Mode, Text, URLPrefix, FilePath, BranchPath string + IsWiki bool +} + // RenderMarkup renders markup text for the /markup and /markdown endpoints -func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPrefix, filePath string, wiki bool) { +func (re *Renderer) RenderMarkup(ctx *context.Base, repo *context.Repository) { var markupType string relativePath := "" - if len(text) == 0 { + if len(re.Text) == 0 { _, _ = ctx.Write([]byte("")) return } - switch mode { + switch re.Mode { case "markdown": // Raw markdown if err := markdown.RenderRaw(&markup.RenderContext{ Ctx: ctx, Links: markup.Links{ AbsolutePrefix: true, - Base: urlPrefix, + Base: re.URLPrefix, }, - }, strings.NewReader(text), ctx.Resp); err != nil { + }, strings.NewReader(re.Text), ctx.Resp); err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) } return @@ -50,30 +55,30 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPr case "file": // File as document based on file extension markupType = "" - relativePath = filePath + relativePath = re.FilePath default: - ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Unknown mode: %s", mode)) + ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Unknown mode: %s", re.Mode)) return } - if !strings.HasPrefix(setting.AppSubURL+"/", urlPrefix) { + if !strings.HasPrefix(setting.AppSubURL+"/", re.URLPrefix) { // check if urlPrefix is already set to a URL linkRegex, _ := xurls.StrictMatchingScheme("https?://") - m := linkRegex.FindStringIndex(urlPrefix) + m := linkRegex.FindStringIndex(re.URLPrefix) if m == nil { - urlPrefix = util.URLJoin(setting.AppURL, urlPrefix) + re.URLPrefix = util.URLJoin(setting.AppURL, re.URLPrefix) } } meta := map[string]string{} if repo != nil && repo.Repository != nil { - if mode == "comment" { + if re.Mode == "comment" { meta = repo.Repository.ComposeMetas(ctx) } else { meta = repo.Repository.ComposeDocumentMetas(ctx) } } - if mode != "comment" { + if re.Mode != "comment" { meta["mode"] = "document" } @@ -81,13 +86,14 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPr Ctx: ctx, Links: markup.Links{ AbsolutePrefix: true, - Base: urlPrefix, + Base: re.URLPrefix, + BranchPath: re.BranchPath, }, Metas: meta, - IsWiki: wiki, + IsWiki: re.IsWiki, Type: markupType, RelativePath: relativePath, - }, strings.NewReader(text), ctx.Resp); err != nil { + }, strings.NewReader(re.Text), ctx.Resp); err != nil { if markup.IsErrUnsupportedRenderExtension(err) { ctx.Error(http.StatusUnprocessableEntity, err.Error()) } else { diff --git a/routers/web/misc/markup.go b/routers/web/misc/markup.go index 2dbbd6fc09..2bae122b91 100644 --- a/routers/web/misc/markup.go +++ b/routers/web/misc/markup.go @@ -14,5 +14,15 @@ import ( // Markup render markup document to HTML func Markup(ctx *context.Context) { form := web.GetForm(ctx).(*api.MarkupOption) - common.RenderMarkup(ctx.Base, ctx.Repo, form.Mode, form.Text, form.Context, form.FilePath, form.Wiki) + + re := common.Renderer{ + Mode: form.Mode, + Text: form.Text, + URLPrefix: form.Context, + FilePath: form.FilePath, + BranchPath: form.BranchPath, + IsWiki: form.Wiki, + } + + re.RenderMarkup(ctx.Base, ctx.Repo) } diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 00c3d880a9..f27ad62982 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -211,6 +211,7 @@ func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() + ctx.Data["BranchPath"] = ctx.Repo.BranchNameSubURL() ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" if canCommit { diff --git a/templates/repo/editor/edit.tmpl b/templates/repo/editor/edit.tmpl index 71cecf1514..39379a977c 100644 --- a/templates/repo/editor/edit.tmpl +++ b/templates/repo/editor/edit.tmpl @@ -28,7 +28,7 @@
{{svg "octicon-code"}} {{if .IsNewFile}}{{ctx.Locale.Tr "repo.editor.new_file"}}{{else}}{{ctx.Locale.Tr "repo.editor.edit_file"}}{{end}} - {{svg "octicon-eye"}} {{ctx.Locale.Tr "preview"}} + {{svg "octicon-eye"}} {{ctx.Locale.Tr "preview"}} {{if not .IsNewFile}} {{svg "octicon-diff"}} {{ctx.Locale.Tr "repo.editor.preview_changes"}} {{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fd147f44a2..c714ff0927 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24322,6 +24322,10 @@ "description": "MarkupOption markup options", "type": "object", "properties": { + "BranchPath": { + "description": "The current branch path where the form gets posted\n\nin: body", + "type": "string" + }, "Context": { "description": "Context to render\n\nin: body", "type": "string" diff --git a/tests/e2e/markdown-editor.test.e2e.ts b/tests/e2e/markdown-editor.test.e2e.ts index 4cdf4644f7..582ea7eceb 100644 --- a/tests/e2e/markdown-editor.test.e2e.ts +++ b/tests/e2e/markdown-editor.test.e2e.ts @@ -11,6 +11,37 @@ test.beforeAll(async ({browser}, workerInfo) => { await login_user(browser, workerInfo, 'user2'); }); +test('Markdown image preview behaviour', async ({browser}, workerInfo) => { + test.skip(workerInfo.project.name === 'Mobile Safari', 'Flaky behaviour on mobile safari;'); + + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + + // Editing the root README.md file for image preview + const editPath = '/user2/repo1/src/branch/master/README.md'; + + const page = await context.newPage(); + const response = await page.goto(editPath, {waitUntil: 'domcontentloaded'}); + expect(response?.status()).toBe(200); + + // Click 'Edit file' tab + await page.locator('[data-tooltip-content="Edit file"]').click(); + + // This yields the monaco editor + const editor = page.getByRole('presentation').nth(0); + await editor.click(); + // Clear all the content + await page.keyboard.press('ControlOrMeta+KeyA'); + // Add the image + await page.keyboard.type('![Logo of Forgejo](./assets/logo.svg "Logo of Forgejo")'); + + // Click 'Preview' tab + await page.locator('a[data-tab="preview"]').click(); + + // Check for the image preview via the expected attribute + const preview = page.locator('div[data-tab="preview"] p[dir="auto"] a'); + await expect(preview).toHaveAttribute('href', 'http://localhost:3003/user2/repo1/media/branch/master/assets/logo.svg'); +}); + test('markdown indentation', async ({browser}, workerInfo) => { const context = await load_logged_in_context(browser, workerInfo, 'user2'); diff --git a/web_src/js/features/repo-editor.js b/web_src/js/features/repo-editor.js index faf8ba1f84..5b59b66f80 100644 --- a/web_src/js/features/repo-editor.js +++ b/web_src/js/features/repo-editor.js @@ -26,6 +26,7 @@ function initEditPreviewTab($form) { const formData = new FormData(); formData.append('mode', mode); formData.append('context', context); + formData.append('branch_path', $this.data('branch-path')); formData.append( 'text', $form.find(`.tab[data-tab="${$tabMenu.data('write')}"] textarea`).val(),