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

Improve the frontend guideline #23298

Merged
merged 4 commits into from
Mar 5, 2023
Merged

Improve the frontend guideline #23298

merged 4 commits into from
Mar 5, 2023

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Mar 4, 2023

The CustomEvent prefix

There was already ce-quick-submit, the ce- prefix seems better than us-. Rename the only us- prefixed us-load-context-popup to ce- prefixed.

Styles and Attributes in Go HTML Template

#21855 (comment)

Suggest to stick to class="c1 {{if $var}}c2{{end}}"

The readability and maintainability should be applied to the code which is read by developers, but not for the generated outputs.

The template code is the code for developers, while the generated HTML are only for browsers.

The class="c1 {{if $var}}c2{{end}}" style is clearer for developers and more intuitive, and the generated HTML also makes browsers happy (a few spaces do not affect anything)

Think about a more complex case:

  • class="{{if $active}}active{{end}} menu item {{if $show}}show{{end}} {{if $warn}}warn{{end}}"
  • --vs--
  • class="{{if $active}}active {{end}}menu item{{if $show}} show{{end}}{{if $warn}} warn{{end}}"

The first style make it clearer to see each CSS class name with its {{if}} block.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 5, 2023
@delvh delvh added the type/docs This PR mainly updates/creates documentation label Mar 5, 2023
@delvh delvh added this to the 1.20.0 milestone Mar 5, 2023
@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 5, 2023
@lunny lunny added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Mar 5, 2023
@yardenshoham yardenshoham added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Mar 5, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #23298 (deb0fe9) into main (e893560) will increase coverage by 0.02%.
The diff coverage is n/a.

📣 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   #23298      +/-   ##
==========================================
+ Coverage   47.56%   47.59%   +0.02%     
==========================================
  Files        1143     1143              
  Lines      151171   151171              
==========================================
+ Hits        71907    71943      +36     
+ Misses      70763    70726      -37     
- Partials     8501     8502       +1     
Impacted Files Coverage Δ
models/asymkey/gpg_key_common.go 52.63% <0.00%> (-3.95%) ⬇️
services/pull/check.go 27.69% <0.00%> (-1.16%) ⬇️
routers/api/v1/repo/pull.go 49.80% <0.00%> (-0.16%) ⬇️
modules/queue/workerpool.go 53.33% <0.00%> (+0.47%) ⬆️
modules/queue/unique_queue_disk_channel.go 66.07% <0.00%> (+1.33%) ⬆️
models/issues/tracked_time.go 65.86% <0.00%> (+1.44%) ⬆️
modules/queue/queue_bytefifo.go 61.01% <0.00%> (+1.80%) ⬆️
modules/log/event.go 60.27% <0.00%> (+2.09%) ⬆️
models/issues/review.go 52.73% <0.00%> (+2.66%) ⬆️
modules/charset/charset.go 75.36% <0.00%> (+3.62%) ⬆️
... and 1 more

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

@lunny lunny merged commit 21a1d76 into go-gitea:main Mar 5, 2023
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 5, 2023
@wxiaoguang wxiaoguang deleted the improve-doc branch March 5, 2023 14:31
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 6, 2023
* giteaofficial/main: (28 commits)
  Update hacking-on-gitea-zh_cn documentation (go-gitea#23315)
  Fix broken code editor diff preview (go-gitea#23307)
  [skip ci] Updated translations via Crowdin
  Add context when rendering labels or emojis (go-gitea#23281)
  Change interactiveBorder to fix popup preview  (go-gitea#23169)
  Improve the frontend guideline (go-gitea#23298)
  Scoped labels: set aria-disabled on muted Exclusive option for a11y (go-gitea#23306)
  Add basic documentation for labels, including scoped labels (go-gitea#23304)
  [skip ci] Updated translations via Crowdin
  Re-add accidentally removed `hacking-on-gitea.zh-cn.md` (go-gitea#23297)
  Add default owner team to privated_org and limited_org in unit test (go-gitea#23109)
  Improve sed detection in update-locales.sh (go-gitea#23254)
  Support sanitising the URL by removing extra slashes in the URL (go-gitea#21333)
  Make Ctrl+Enter submit a pending comment (starting review) instead of submitting a single comment (go-gitea#23245)
  Avoid panic caused by broken payload when creating commit status (go-gitea#23216)
  Add run status in action view page (go-gitea#23212)
  update to mermaid v10 (go-gitea#23178)
  Fix code wrap for unbroken lines (go-gitea#23268)
  Fix stray backticks appearing in pull request timeline (go-gitea#23282)
  Fill head commit to in payload when notifying push commits for mirroring (go-gitea#23215)
  ...
@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
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants