Skip to content
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 br display for packages curls #23737

Merged
merged 5 commits into from Mar 28, 2023

Conversation

HesterG
Copy link
Contributor

@HesterG HesterG commented Mar 27, 2023

Before:
截屏2023-03-27 15 48 23
This happens because the <br> matches this rule, which is not necessary here (This is introduced by #22861, did a quick check, and this is the only place used <br> inside <code> from the PR):

.markup code br,
.markup tt br {
  display: none;
}

After:
截屏2023-03-27 15 46 50

@wolfogre wolfogre added type/bug topic/ui Change the appearance of the Gitea UI outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 27, 2023
@wolfogre wolfogre added this to the 1.20.0 milestone Mar 27, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2023

Codecov Report

Merging #23737 (632bfba) into main (f521e88) will increase coverage by 0.00%.
The diff coverage is 41.65%.

❗ Current head 632bfba differs from pull request most recent head 1e16e9e. Consider uploading reports for the commit 1e16e9e to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##             main   #23737     +/-   ##
=========================================
  Coverage   47.14%   47.14%             
=========================================
  Files        1149     1155      +6     
  Lines      151446   152485   +1039     
=========================================
+ Hits        71397    71893    +496     
- Misses      71611    72112    +501     
- Partials     8438     8480     +42     
Impacted Files Coverage Δ
cmd/dump.go 0.66% <0.00%> (-0.01%) ⬇️
cmd/web.go 0.00% <0.00%> (ø)
models/actions/run.go 1.64% <0.00%> (-0.08%) ⬇️
models/actions/runner.go 1.44% <ø> (ø)
models/packages/package.go 45.45% <0.00%> (-1.13%) ⬇️
models/user/search.go 77.50% <0.00%> (-6.29%) ⬇️
modules/actions/workflows.go 0.00% <0.00%> (ø)
modules/context/context.go 64.54% <0.00%> (-3.53%) ⬇️
modules/doctor/storage.go 30.65% <0.00%> (-1.29%) ⬇️
modules/setting/git.go 45.45% <ø> (ø)
... and 38 more

... and 45 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -6,7 +6,7 @@
<label>{{svg "octicon-terminal"}} {{.locale.Tr "packages.generic.download"}}</label>
<div class="markup"><pre class="code-block"><code>
{{- range .PackageDescriptor.Files -}}
curl <gitea-origin-url data-url="{{AppSubUrl}}/api/packages/{{$.PackageDescriptor.Owner.Name}}/generic/{{$.PackageDescriptor.Package.Name}}/{{$.PackageDescriptor.Version.Version}}/{{.File.Name}}"></gitea-origin-url><br>
curl <gitea-origin-url data-url="{{AppSubUrl}}/api/packages/{{$.PackageDescriptor.Owner.Name}}/generic/{{$.PackageDescriptor.Package.Name}}/{{$.PackageDescriptor.Version.Version}}/{{.File.Name}}"></gitea-origin-url><br class="gt-di">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look good to me ....

I would say that the <br> was polluted somewhere else.

Couldn't the pollution be fixed properly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@silverwind

.markup code br,
.markup tt br {
  display: none;
}

Why do we need these styles?

Copy link
Contributor Author

@HesterG HesterG Mar 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look good to me ....

I would say that the <br> was polluted somewhere else.

Couldn't the pollution be fixed properly?

Added a commit to get rid of the <br> here as mentioned in the comment below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need these styles?

These do come from GitHub's CSS.

IDK their significance but I assume inside <code> and such, one breaks via \n usually because of white-space: pre formatting, not <br>.

@wxiaoguang
Copy link
Contributor

To avoid using br, I guess you can do this:

{{- range .PackageDescriptor.Files -}}
curl <gitea-origin-url data-url="{{AppSubUrl}}/api/packages/{{$.PackageDescriptor.Owner.Name}}/generic/{{$.PackageDescriptor.Package.Name}}/{{$.PackageDescriptor.Version.Version}}/{{.File.Name}}"></gitea-origin-url>
{{end -}}

Just copy the code, no space indent, the last is {{end -}}

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 28, 2023
@techknowlogick
Copy link
Member

ping lg-tm

@techknowlogick techknowlogick merged commit f401337 into go-gitea:main Mar 28, 2023
2 checks passed
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 28, 2023
Before:
<img width="1403" alt="截屏2023-03-27 15 48 23"
src="https://user-images.githubusercontent.com/17645053/227875392-399debf7-db75-4d9a-9436-409f75447c65.png">
This happens because the `<br>` matches this
[rule](https://github.com/go-gitea/gitea/blob/e6e602fd8d35471f1e2f4a42669a1f17e76e0176/web_src/css/markup/content.css#L428),
which is not necessary here (This is introduced by go-gitea#22861, did a quick
check, and this is the only place used `<br>` inside `<code>` from the
PR):
```css
.markup code br,
.markup tt br {
  display: none;
}
```

After:
<img width="1398" alt="截屏2023-03-27 15 46 50"
src="https://user-images.githubusercontent.com/17645053/227875244-b7fba432-b32c-42f7-9517-4e05bb2e64ea.png">

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Mar 28, 2023
zeripath pushed a commit that referenced this pull request Mar 28, 2023
Backport #23737 by @HesterG

Before:
<img width="1403" alt="截屏2023-03-27 15 48 23"
src="https://user-images.githubusercontent.com/17645053/227875392-399debf7-db75-4d9a-9436-409f75447c65.png">
This happens because the `<br>` matches this
[rule](https://github.com/go-gitea/gitea/blob/e6e602fd8d35471f1e2f4a42669a1f17e76e0176/web_src/css/markup/content.css#L428),
which is not necessary here (This is introduced by #22861, did a quick
check, and this is the only place used `<br>` inside `<code>` from the
PR):
```css
.markup code br,
.markup tt br {
  display: none;
}
```

After:
<img width="1398" alt="截屏2023-03-27 15 46 50"
src="https://user-images.githubusercontent.com/17645053/227875244-b7fba432-b32c-42f7-9517-4e05bb2e64ea.png">

Co-authored-by: Hester Gong <hestergong@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@wxiaoguang wxiaoguang deleted the fix-br-display-package branch March 28, 2023 08:02
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 29, 2023
* upstream/main:
  Refactor internal API for git commands, use meaningful messages instead of "Internal Server Error" (go-gitea#23687)
  Add CSS rules for basic colored labels (go-gitea#23774)
  Add meilisearch support (go-gitea#23136)
  Add missing translation for `actions.runners.reset_registration_token_success` (go-gitea#23732)
  [skip ci] Updated translations via Crowdin
  Implement Issue Config (go-gitea#20956)
  Set repository link based on the url in package.json for npm packages (go-gitea#20379)
  Add API to manage issue dependencies (go-gitea#17935)
  Add creation time in tag list page (go-gitea#23693)
  Make minio package support legacy MD5 checksum (go-gitea#23768)
  Yarden Shoham has a new email address (go-gitea#23767)
  fix br display for packages curls (go-gitea#23737)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 topic/packages topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants