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

Toasts: Overlay close button on corner #480

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

alainm23
Copy link

@alainm23 alainm23 commented Mar 21, 2021

Fixes #378.

image

@hanaral
Copy link

hanaral commented Apr 6, 2021

Noice, I think it goes far better with the elementary style, and doesn't take up needless space on something that self-dismisses anyway.
Edit: Tagging @cassidyjames for a review
Edit 2: This also requires the style of badge to be implemented in the stylesheet (it's invisible otherwise)

@ryonakano ryonakano requested a review from a team April 6, 2021 23:31
Copy link

@hanaral hanaral left a comment

Choose a reason for hiding this comment

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

Might want to bump copyright while you're at it

Copy link
Sponsor Contributor

@ryonakano ryonakano left a comment

Choose a reason for hiding this comment

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

Not tested yet but leaving some nitpicking coding style review.

lib/Widgets/Toast.vala Outdated Show resolved Hide resolved
lib/Widgets/Toast.vala Outdated Show resolved Hide resolved
ryonakano and others added 4 commits April 7, 2021 09:24
Co-authored-by: Ryo Nakano <26003928+ryonakano@users.noreply.github.com>
Co-authored-by: Ryo Nakano <26003928+ryonakano@users.noreply.github.com>
@cassidyjames cassidyjames changed the title Fix #378 Toasts: Overlay close button on corner Apr 20, 2021
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

This PR does not seem to do the styling of the close button? When I tried it the close button was not in the required style so not really usable. Other than that this, the code LGTM and works as expected.

Should this be blocked pending amendments to the stylesheet being merged?

@hanaral
Copy link

hanaral commented Jul 26, 2021

@alainm23 Any chance you could try fix the problem so it can be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redesign idea for Widgets.Toast
5 participants