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.

Sign up for GitHub

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

Jump to bottom

Add tooltips to Hide comment type settings where necessary #21306

Merged
merged 11 commits into from
Apr 12, 2023
Merged

Add tooltips to Hide comment type settings where necessary #21306

merged 11 commits into from
Apr 12, 2023

Conversation

delvh
Copy link
Member

@delvh delvh commented Sep 30, 2022 •
edited
Loading

Previously, this setting was pretty confusing for users, especially the difference between "reference" and "issue reference".
Related: #21321.

@delvh
Add tooltips to "Hide comment type" settings where necessary
197164a 
Previously, this setting was pretty confusing for users, especially the
difference between "reference" and "issue reference".
@delvh delvh added the topic/ui-interaction Change the process how users use Gitea instead of the visual appearance label Sep 30, 2022
@delvh delvh added this to the 1.18.0 milestone Sep 30, 2022
@delvh
Copy link
Member Author

delvh commented Sep 30, 2022

Review hint: Hide whitespace changes.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 30, 2022
@delvh delvh mentioned this pull request Oct 2, 2022
Open
@lunny
Copy link
Member

lunny commented Oct 24, 2022

image

The position of the tooltip is wired.

@delvh
Copy link
Member Author

delvh commented Oct 24, 2022

I didn't change the positioning of the tooltip.
As far as I know, tippy automatically detects whether to position on the top or the bottom depending on where you hover over the box.
Meaning when you enter the box from the bottom and then hover there, it should be positioned on the bottom.
Or it positions it on the top otherwise.

@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 26, 2022
@lunny lunny modified the milestones: 1.19.0, 1.20.0 Feb 3, 2023
@silverwind
Copy link
Member

Tippy may sometimes try to be smart about positioning, but you can add data-tooltip-placement="top" to force it on top.

@delvh delvh changed the title Add tooltips to "Hide comment type" settings where necessary Add tooltips to Hide comment type settings where necessary Apr 10, 2023
@delvh
Copy link
Member Author

delvh commented Apr 10, 2023

ping for maintainers.
This PR is up to date, and would be nice to have as it helps against the current confusion.

@codecov-commenter

This comment was marked as off-topic.

Sign in to view
@lunny
Copy link
Member

lunny commented Apr 11, 2023

I couldn't know why we need a tooltip here. Maybe just some text on top of all checkboxes is good enough?

image

Like this
image

@delvh
Copy link
Member Author

delvh commented Apr 11, 2023

Related: #21321.

That's exactly my point why I opened this issue.
There's simply no guideline yet on when to use what.
I agree, for the topmost tooltip, a static help text is probably the best.
But not for the ones below.

@delvh
Copy link
Member Author

delvh commented Apr 11, 2023

@lunny applied.

lunny
lunny approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Have a test with no problem from UI.

@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 12, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 12, 2023
@wxiaoguang
Copy link
Contributor

Please wait for these comments. Especially #21306 (comment) , I do not think the HTML layout is correct.

@delvh
Copy link
Member Author

delvh commented Apr 12, 2023

@wxiaoguang @lunny all done.

@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 Apr 12, 2023
@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 12, 2023
@lunny lunny merged commit 58b36cc into go-gitea:main Apr 12, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 12, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 13, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
a3da958 
* upstream/main:
  [skip ci] Updated translations via Crowdin
  Bump vm2 from 3.9.15 to 3.9.16 (go-gitea#24089)
  Fix difficult translation for other languages (go-gitea#24070)
  Fix mismatch between hook events and github event types (go-gitea#24048)
  Improve LFS error logs (go-gitea#24072)
  Update the value of the `diffEnd` when click `Show More` btn in the DiffFileTree (go-gitea#24069)
  Add tooltips to `Hide comment type` settings where necessary (go-gitea#21306)
  Use a general approach to access custom/static/builtin assets (go-gitea#24022)
  Make Release Download URLs predictable (go-gitea#23891)
  Expand selected file when clicking file tree (go-gitea#24041)
  Localize milestone related time strings (go-gitea#24051)
  update BSDmakefile to latest version from upstream (go-gitea#24063)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
@delvh delvh deleted the feature/document-comment-type-hiding branch October 19, 2023 20:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@HesterG HesterG HesterG approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui-interaction Change the process how users use Gitea instead of the visual appearance
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
7 participants
@delvh @lunny @silverwind @codecov-commenter @wxiaoguang @HesterG @GiteaBot