Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Make toast support preventDuplicates #31501

Merged
merged 15 commits into from
Jun 27, 2024
Merged

Make toast support preventDuplicates #31501

merged 15 commits into from
Jun 27, 2024

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Jun 26, 2024 •
edited by silverwind
Loading

make preventDuplicates default to true, users get a clear UI feedback and know that "a new message appears".

Fixes: #26651

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 26, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 26, 2024
@wxiaoguang wxiaoguang mentioned this pull request Jun 26, 2024
Closed
@github-actions github-actions bot added modifies/templates This PR modifies the template files modifies/js labels Jun 26, 2024
@wxiaoguang wxiaoguang added the type/enhancement An improvement of existing functionality label Jun 26, 2024
@silverwind
Copy link
Member

silverwind commented Jun 26, 2024 •
edited
Loading

we should hide the existing toast and show the new toast

As I said, I think this is suboptimal UX because it makes it harder then necessary to copy text out a toast that re-creates for example every 1-2 seconds. I think it's better to have a counter instead that shows the number of times a toast has triggered. Could be as simple as appending (2) to the message. Same mechanism we alredy have in place in the global js error handler.

@wxiaoguang
Copy link
Contributor Author

Could be as simple as appending (2) to the message.

f6034b2

@silverwind
Copy link
Member

silverwind commented Jun 26, 2024 •
edited
Loading

Looks quite good but I noticed that the animation only plays when it goes from 1 to 2, but not from 2 to 3 or later, is it intentional?

@wxiaoguang @silverwind
Update web_src/css/modules/toast.css
0b5a4bc 
Co-authored-by: silverwind <me@silverwind.io>
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 26, 2024 •
edited
Loading

Looks quite good but I noticed that the animation only plays when it goes from 1 to 2, but not from 2 to 3 or later, is it intentional?

It works on my side, and the code seems right.

@silverwind
Copy link
Member

silverwind commented Jun 26, 2024 •
edited
Loading

Subsequent animations work for me only in Chrome, not in Firefox.

@silverwind
Copy link
Member

Could you add to devtest a second row of buttons that generates different messages?

@wxiaoguang
Copy link
Contributor Author

Subsequent animations work for me only in Chrome, not in Firefox.

6a9ed8c

@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 Jun 26, 2024
@yp05327
Copy link
Contributor

yp05327 commented Jun 27, 2024 •
edited
Loading

Nice fix. Can you add fix #26651 in the description.

@yp05327
Copy link
Contributor

yp05327 commented Jun 27, 2024 •
edited
Loading

Maybe we can add more spaces here? Then the animation seems better.
image

@yp05327 yp05327 added the topic/ui Change the appearance of the Gitea UI label Jun 27, 2024
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 27, 2024
@silverwind
Copy link
Member

Nice fix. Can you add fix #26651 in the description.

Added

@yp05327
Copy link
Contributor

yp05327 commented Jun 27, 2024 •
edited
Loading

before:
image
after:
image
It seems that the icon is too big?
But I can't see this in @wxiaoguang 's screenshot 🤔

@yp05327
Copy link
Contributor

yp05327 commented Jun 27, 2024 •
edited
Loading

image
image

If you scale the screen to 300%, you can see it clearly.
image

ps: same to another issue that caused by different OS and browser?

@wxiaoguang
Copy link
Contributor Author

I don't want to play with the pixel-level tricks here.

The UI doesn't really look problematic and it doesn't cause problems. I think it could be fine-tuned later.

@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 Jun 27, 2024
@silverwind
Copy link
Member

silverwind commented Jun 27, 2024 •
edited
Loading

Not sure if vertical alignment of first line is easily possible because the text can be multiline.

@wxiaoguang wxiaoguang enabled auto-merge (squash) June 27, 2024 13:43
@wxiaoguang wxiaoguang merged commit c1fe6fb into go-gitea:main Jun 27, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jun 27, 2024
@wxiaoguang wxiaoguang deleted the fix-toast branch June 27, 2024 13:59
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 28, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
b9bbd74 
* giteaofficial/main:
  Fix avatar radius problem on the new issue page (go-gitea#31506)
  Make toast support preventDuplicates (go-gitea#31501)
  Improve attachment upload methods (go-gitea#30513)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 26, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@silverwind silverwind silverwind approved these changes

@yp05327 yp05327 yp05327 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. modifies/js modifies/templates This PR modifies the template files size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

Duplicate check and an auto disappear timer for toast
4 participants
@wxiaoguang @silverwind @yp05327 @GiteaBot