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

Add title attribute to dependencies in sidebar #19807

Conversation

zeripath
Copy link
Contributor

Add the full title as the title attribute on dependencies in
the sidebar.

Fix #19806

Signed-off-by: Andrew Thornton art27@cantab.net

Add the full title as the title attribute on dependencies in
the sidebar.

Fix go-gitea#19806

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/bug topic/ui Change the appearance of the Gitea UI labels May 25, 2022
@zeripath zeripath added this to the 1.17.0 milestone May 25, 2022
@silverwind
Copy link
Member

I think class tooltip with data-content should be prefered over title.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 26, 2022
@delvh
Copy link
Member

delvh commented May 26, 2022

As far as I can see:
The only thing speaking against title is its release date:
It seems to have been accepted into the standard not too long ago, resulting in only being usable on about 55% of all devices.
For the future (say perhaps in three years), I think we should abandon using tooltip with data-content for using title instead as it makes more sense to use the browser-native version than one depending on external JS.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 27, 2022
Copy link

@hello-smile6 hello-smile6 left a comment

Choose a reason for hiding this comment

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

Would approve, but I can't.

@silverwind
Copy link
Member

As far as I can see: The only thing speaking against title is its release date: It seems to have been accepted into the standard not too long ago, resulting in only being usable on about 55% of all devices. For the future (say perhaps in three years), I think we should abandon using tooltip with data-content for using title instead as it makes more sense to use the browser-native version than one depending on external JS.

title attribute is not a new browser features, it' ancient. It can not be styled and at least in Firefox, it results in a tiny text, with like 10px font size which is borderline unreadable to me, that's why I advocate JS-based tooltips. While there are also CSS-based tooltips, those have far too many issues with z-index and such and I would not consider them.

@delvh
Copy link
Member

delvh commented May 27, 2022

title attribute is not a new browser features, it' ancient.

Ancient?
Why do both Firefox and Chrome only support it since late April 2022 then?
The only browsers it is ancient for are IE and Edge...
(https://caniuse.com/mdn-html_global_attributes_title)

It can not be styled and at least in Firefox, it results in a tiny text, with like 10px font size which is borderline unreadable to me, that's why I advocate JS-based tooltips. While there are also CSS-based tooltips, those have far too many issues with z-index and such and I would not consider them.

That, however, is a valid reason, even though I think this is one of the things, we actually don't want to style ourselves...
The browsers should know themselves what a good default value for it is.

@silverwind
Copy link
Member

silverwind commented May 27, 2022

Why do both Firefox and Chrome only support it since late April 2022 then?

Seems like inaccurate CanIUse data, the MDN table is better:

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/title#browser_compatibility

title attribute dates back to Netscape days I think, it's really ancient. Example reference from 1998:

https://people.apache.org/~jim/NewArchitect/webrevu/tag/1998/04_03_98.html

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Better than before, but I'd still prefer all title usage to be refactored to use tooltip later. It's just better UX than title.

@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 May 28, 2022
@zeripath
Copy link
Contributor Author

zeripath commented Jun 1, 2022

Better than before, but I'd still prefer all title usage to be refactored to use tooltip later. It's just better UX than title.

OK done.

@techknowlogick techknowlogick merged commit bbffdda into go-gitea:main Jun 1, 2022
@zeripath zeripath deleted the fix-19806-add-title-attribute-to-dependency-lists branch June 1, 2022 19:18
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 2, 2022
* giteaofficial/main:
  update documents (go-gitea#19868)
  Only return valid stopwatches (go-gitea#19863)
  [skip ci] Updated translations via Crowdin
  Add title attribute to dependencies in sidebar (go-gitea#19807)
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* Add title attribute to dependencies in sidebar

Add the full title as the title attribute on dependencies in
the sidebar.

Fix go-gitea#19806

Signed-off-by: Andrew Thornton <art27@cantab.net>

* as per silverwind

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@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/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

can not read the full title of issues in dependency list of an issue
7 participants