Improve RSS feed icons #28368

n0toose · 2023-12-06T04:47:22Z

The RSS Feed icons were placed in a proper button, so that it does
not look "inconsistent". This also makes the problem of the button
being improperly aligned go away.
The icon that shows on user profiles has not been modified because
of a lack of better implementation ideas.
Where applicable, the RSS Feed icon was put directly next to the
Follow button (right menu), as both functionalities effectively
share the same purpose.
Despite the attempt at achieving less inconsistency, a conscious
decision to not add any text to those buttons was made, opting for
tooltips instead. "Make it present, but not too annoying."
A special exception was made for the Releases pages (which contains
text, not a tooltip), where an RSS feed would be particularly
beneficial to users.

The fact that the RSS functionality is explicitly optional was taken
into account, and these improvements were made with public-facing
instances (where the feature works best) in mind.

- The RSS Feed icons were placed in a proper button, so that it does not look "inconsistent". This also makes the problem of the button being improperly aligned go away. - The icon that shows on user profiles has not been modified because of a lack of better implementation ideas. - Where applicable, the RSS Feed icon was put directly next to the Follow button (right menu), as both functionalities effectively share the same purpose. - Despite the attempt at achieving less inconsistency, a conscious decision to not add any text to those buttons was made, opting for tooltips instead. "Make it present, but not too annoying." - A special exception was made for the Releases pages (which contains text, not a tooltip), where an RSS feed would be particularly beneficial to users. The fact that the RSS functionality is explicitly optional was taken into account, and these improvements were made with public-facing instances (where the feature works best) in mind.

n0toose · 2023-12-06T04:50:09Z

Screenshots

(Note: The tooltip "RSS Feed" still appears when hovering over the button. In the repository header, the RSS feed button was consciously made slightly wider.

Organization

Repository

Releases

(logged in)

templates/repo/release_tag_header.tmpl

wxiaoguang · 2023-12-07T00:36:43Z

templates/org/home.tmpl

@@ -23,6 +20,11 @@
 			</div>
 		</div>
 		<div class="right menu">
+			{{if .EnableFeed}}
+			<button class="link-action ui basic label button gt-mr-0" data-tooltip-content="{{ctx.Locale.Tr "rss_feed"}}" data-url="{{$.Org.HomeLink}}.rss">


Sorry but what's this? What's the purpose of link-action + data-url for the RSS link?

It breaks the link.

wxiaoguang · 2023-12-07T00:37:34Z

templates/repo/header.tmpl

@@ -55,6 +52,12 @@
 							</div>
 						</form>
 					{{end}}
+					{{if $.EnableFeed}}
+					{{/* An extra div-element is not necessary here, as this button does not secretly contain two buttons. */}}
+					<button class="ui compact small basic button" data-url="{{$.RepoLink}}.rss" data-tooltip-content="{{ctx.Locale.Tr "rss_feed"}}">


Again, has this code been tested?

I do not think data-url would work for a RSS link.

Gosh, I got so preoccupied with trying to make the icons look consistent enough UI-wise to the point where I may have overlooked the possibility that such an attempt could possibly break the URL.

And here I got super happy that I managed to get a Gitea change in without a change requested or breaking things - I understand that this isn't the first time... I will spend the next few minutes looking into this, issuing a fix and if I don't manage it, I'll send in a Revert and then start over.

I think you could try to use <a class="...." href="{{the RSS link}}">...</a>.

<button> means it requires extra JS code to work with a link or it just submits a form.

link-action means it is an AJAX POST request by data-url.

I don't want to take more of your time, so I'll inform you of the following: The faulty behavior seems to come from a botched very-early-in-the-morning rebase. I'll need like 30 minutes to properly fix this and then send a subsequent review that fixes this.

I think you could try to use ....

Yep, that was my plan. I just made a commit that does this exactly (and explicitly defines role=button so that it works under screen readers), but I'll need a few extra minutes to fully test this.

In the future, I'll try to attach short clips after deciding that the commit is PR ready for self-verification reasons and as well as to ease the review process / cause less stress to maintainers.

Follow-up for go-gitea#28368 - Just replace button with an a-element with the button class - Use role=button for screen readers - Remove useless link-action class from template/org/home.tmpl

Follow-up for #28368 - Just replace button with an a-element with the button class - Remove useless link-action class from template/org/home.tmpl

* giteaofficial/main: Improve text in Security settings (go-gitea#28393) Fix Docker meta action for releases (go-gitea#28232) Make gogit Repository.GetBranchNames consistent (go-gitea#28348) Remove GetByBean method because sometimes it's danger when query condition parameter is zero and also introduce new generic methods (go-gitea#28220) Include public repos in doer's dashboard for issue search (go-gitea#28304) Issue fixes for RSS feed improvements (go-gitea#28380) Fix margin in server signed signature verification view (go-gitea#28379) [skip ci] Updated translations via Crowdin Fix incorrect run order of action jobs (go-gitea#28367) Improve RSS feed icons (go-gitea#28368) Use `filepath` instead of `path` to create SQLite3 database file (go-gitea#28374) Fix incorrect default value of `[attachment].MAX_SIZE` (go-gitea#28373) Fix object does not exist error when checking citation file (go-gitea#28314)

- The RSS Feed icons were placed in a proper button, so that it does not look "inconsistent". This also makes the problem of the button being improperly aligned go away. - The icon that shows on user profiles has not been modified because of a lack of better implementation ideas. - Where applicable, the RSS Feed icon was put directly next to the Follow button (right menu), as both functionalities effectively share the same purpose. - Despite the attempt at achieving less inconsistency, a conscious decision to not add any text to those buttons was made, opting for tooltips instead. "Make it present, but not too annoying." - A special exception was made for the Releases pages (which contains text, not a tooltip), where an RSS feed would be particularly beneficial to users. The fact that the RSS functionality is explicitly optional was taken into account, and these improvements were made with public-facing instances (where the feature works best) in mind.

Follow-up for go-gitea#28368 - Just replace button with an a-element with the button class - Remove useless link-action class from template/org/home.tmpl

- The RSS Feed icons were placed in a proper button, so that it does not look "inconsistent". This also makes the problem of the button being improperly aligned go away. - The icon that shows on user profiles has not been modified because of a lack of better implementation ideas. - Where applicable, the RSS Feed icon was put directly next to the Follow button (right menu), as both functionalities effectively share the same purpose. - Despite the attempt at achieving less inconsistency, a conscious decision to not add any text to those buttons was made, opting for tooltips instead. "Make it present, but not too annoying." - A special exception was made for the Releases pages (which contains text, not a tooltip), where an RSS feed would be particularly beneficial to users. The fact that the RSS functionality is explicitly optional was taken into account, and these improvements were made with public-facing instances (where the feature works best) in mind.

Follow-up for go-gitea#28368 - Just replace button with an a-element with the button class - Remove useless link-action class from template/org/home.tmpl

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 6, 2023

pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 6, 2023

github-actions bot added the modifies/templates This PR modifies the template files label Dec 6, 2023

delvh reviewed Dec 6, 2023

View reviewed changes

templates/repo/release_tag_header.tmpl Show resolved Hide resolved

delvh approved these changes Dec 6, 2023

View reviewed changes

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 Dec 6, 2023

delvh added the topic/ui-interaction Change the process how users use Gitea instead of the visual appearance label Dec 6, 2023

jolheiser approved these changes Dec 6, 2023

View reviewed changes

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 Dec 6, 2023

jolheiser enabled auto-merge (squash) December 6, 2023 20:04

jolheiser added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 6, 2023

Merge branch 'main' into rss-feed-icon

68df4e1

jolheiser merged commit 22cb5b0 into go-gitea:main Dec 6, 2023
24 checks passed

GiteaBot added this to the 1.22.0 milestone Dec 6, 2023

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 6, 2023

wxiaoguang reviewed Dec 7, 2023

View reviewed changes

n0toose mentioned this pull request Dec 7, 2023

Issue fixes for RSS feed improvements #28380

Merged

lunny pushed a commit that referenced this pull request Dec 7, 2023

Issue fixes for RSS feed improvements (#28380)

dac7728

Follow-up for #28368 - Just replace button with an a-element with the button class - Remove useless link-action class from template/org/home.tmpl

go-gitea locked as resolved and limited conversation to collaborators Mar 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve RSS feed icons #28368

Improve RSS feed icons #28368

n0toose commented Dec 6, 2023

n0toose commented Dec 6, 2023 •

edited

Loading

wxiaoguang Dec 7, 2023

wxiaoguang Dec 7, 2023

n0toose Dec 7, 2023 •

edited

Loading

wxiaoguang Dec 7, 2023

n0toose Dec 7, 2023

n0toose Dec 7, 2023 •

edited

Loading

Improve RSS feed icons #28368

Improve RSS feed icons #28368

Conversation

n0toose commented Dec 6, 2023

n0toose commented Dec 6, 2023 • edited Loading

Screenshots

Organization

Repository

Releases

wxiaoguang Dec 7, 2023

Choose a reason for hiding this comment

wxiaoguang Dec 7, 2023

Choose a reason for hiding this comment

n0toose Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Dec 7, 2023

Choose a reason for hiding this comment

n0toose Dec 7, 2023

Choose a reason for hiding this comment

n0toose Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

n0toose commented Dec 6, 2023 •

edited

Loading

n0toose Dec 7, 2023 •

edited

Loading

n0toose Dec 7, 2023 •

edited

Loading