Skip to content

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Mar 23, 2023

Follow:

Major changes:

  • Remove "tooltip" class, use "[data-tooltip-content=...]" instead of ".tooltip[data-content=...]"
  • Remove legacy data-position, it's dead code since last Fomantic Tooltip -> Tippy Tooltip refactoring
  • Rename reaction attribute from data-content to data-reaction-content
  • Add comments for some data-content: {{/* used by the form */}}
  • Remove empty "ui" class
  • Use "text color" for SVG icons (a few)

@wxiaoguang wxiaoguang force-pushed the refactor-tooltip-content branch from c775700 to 1905b8b Compare March 23, 2023 05:25
# Conflicts:
#	templates/repo/sub_menu.tmpl
@lunny lunny added topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Mar 23, 2023
@wxiaoguang wxiaoguang force-pushed the refactor-tooltip-content branch from 2083ad8 to 9c388a9 Compare March 23, 2023 13:31
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

I do wonder if we should add tippy to elements with title attributes - so that these get rendered in the same style.

I also wonder if we should just make this data-tooltip instead of data-tooltip-content because the content here seems unnecessary.

However, LGTM even without those.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 23, 2023
@wxiaoguang
Copy link
Contributor Author

I do wonder if we should add tippy to elements with title attributes - so that these get rendered in the same style.

No, sometimes, developers just want to use the native tooltip, give them the chance. Otherwise, the "title" attribute gets polluted. For example: #23574 (comment)

I also wonder if we should just make this data-tooltip instead of data-tooltip-content because the content here seems unnecessary.

I would say it is necessary.

  1. It matches the code tippy.setContent, it's just for content
  2. data-tooltip conlifcts with Fomantic Tooltip, if you do a global search in project, you can find many data-tooltip in Fomantic Build directory. Although the Fomantic Tooltip had been dropped, I still think avoiding conflicts is good.

However, LGTM even without those.

Thank you very much.

@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 24, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 24, 2023
@lunny lunny merged commit 8d5fbeb into go-gitea:main Mar 24, 2023
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 24, 2023
@wxiaoguang wxiaoguang deleted the refactor-tooltip-content branch March 24, 2023 12:38
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 27, 2023
* upstream/main: (23 commits)
  Fix project card preview select and template select (go-gitea#23684)
  [skip ci] Updated translations via Crowdin
  Add git dashes separator to some "log" and "diff" commands (go-gitea#23606)
  Add Simplified Chinese translate for oauth2-provider (go-gitea#23713)
  Fix incorrect `toggle` buttons (go-gitea#23676)
  Fine tune more downdrop settings, use SVG for labels, improve Repo Topic Edit form (go-gitea#23626)
  Allow new file and edit file preview if it has editable extension (go-gitea#23624)
  [skip ci] Updated translations via Crowdin
  Clean some legacy files and move some build files (go-gitea#23699)
  Remove row clicking from notification table (go-gitea#22695)
  Describe Gitea's purpose more accurately (go-gitea#23698)
  [skip ci] Updated translations via Crowdin
  ensure go/bin path exists when copying hugo bin into it (go-gitea#23692)
  Create commit status when event is `pull_request_sync` (go-gitea#23683)
  Add `deps-docs` command to makefile (go-gitea#23686)
  Fix incorrect package doc link (go-gitea#23679)
  Improve indices for `action` table (go-gitea#23532)
  Clarify that Gitea requires JavaScript (go-gitea#23677)
  Use data-tooltip-content for tippy tooltip (go-gitea#23649)
  Add aria attributes to interactive time tooltips. (go-gitea#23661)
  ...
@delvh delvh added this to the 1.20.0 milestone Apr 3, 2023
@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. topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants