appview/pages: PR comment link button should specify round #1187

Using a SSR link too so that it can't be overridden by any custom browser plugin. Not the best way of doing things but works.

+4 -1

2 changed files

Diff round #0

expand all

appview

pages

templates

fragments

line-quote-button.html

repo

pulls

pull.html

+3 -1

appview/pages/templates/fragments/line-quote-button.html

··· 227 227 ? firstAnchor 228 228 : `${firstAnchor}~${lastAnchor}`; 229 229 230 - const md = `[\`${label}\`](${window.location.pathname}${window.location.search}#${fragment})`; 230 + const linkBase = document.getElementById('round-link-base')?.value 231 + || (window.location.pathname + window.location.search); 232 + const md = `[\`${label}\`](${linkBase}#${fragment})`; 231 233 232 234 const { selectionStart: s, selectionEnd: end, value } = ta; 233 235 const before = value.slice(0, s);

appview/pages/templates/repo/pulls/pull.html

··· 99 99 {{ end }} 100 100 101 101 {{ define "contentAfter" }} 102 + <input type="hidden" id="round-link-base" value="/{{ .RepoInfo.FullName }}/pulls/{{ .Pull.PullId }}/round/{{ .ActiveRound }}" /> 102 103 {{ template "repo/fragments/diff" (list .Diff .DiffOpts $) }} 103 104 {{ end }} 104 105

History

1 round 5 comments

oyster.cafe submitted #0 3d

1 commit

expand

c9972bed

appview/pages: link quote button specify round

3/3 success

expand

expand 5 comments

oppi.li 3d

appview/pages/templates/fragments/line-quote-button.html:231 why don't we do current URL - existing fragment + new fragment?

would that result in malformed links in some cases perhaps (multiline etc.)?

oyster.cafe 3d

I think both achieve exactly the same thing, I just thought this way would look a little more explicit

oppi.li 3d

using current URL approach would avoid the need for the hidden input with the active round number (the active round will be encoded in the URL always).

oyster.cafe 3d

if I did that then there'd be no point to the PR I think, the point for me is to render the round server side so that there's no chance of getting to some magic PR state client side where my window.location doesn't have the round path, like my comments in https://tangled.org/tangled.org/core/pulls/1182/round/2

oppi.li 3d

okay! lgtm!

pull request successfully merged