Skip to content

feat: support message dialogs with 3 buttons #2641

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

Open
wants to merge 14 commits into
base: v2
Choose a base branch
from

Conversation

amrbashir
Copy link
Member

closes #2640

@amrbashir amrbashir requested a review from a team as a code owner April 17, 2025 03:13
Copy link
Contributor

github-actions bot commented Apr 17, 2025

Package Changes Through e3fc76e

There are 7 changes which include dialog with minor, dialog-js with minor, single-instance with patch, fs with minor, fs-js with minor, http with patch, http-js with patch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
api-example 2.0.25 2.0.26
api-example-js 2.0.21 2.0.22
fs 2.2.1 2.3.0
fs-js 2.2.1 2.3.0
dialog 2.2.1 2.3.0
dialog-js 2.2.1 2.3.0
http 2.4.3 2.4.4
http-js 2.4.3 2.4.4
persisted-scope 2.2.1 2.2.2
single-instance 2.2.3 2.2.4

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@amrbashir
Copy link
Member Author

iOS is not tested ofc

Windows and Android are tested and works.

@amrbashir amrbashir requested a review from Legend-Master April 19, 2025 11:34
@amrbashir
Copy link
Member Author

@FabianLars or @lucasfernog can you test the iOS stuff?

@FabianLars
Copy link
Member

Can do but I'm kinda away over the weekend so please remind me on Monday if I or Lucas didn't get to it yet by then.

@amrbashir
Copy link
Member Author

don't worry about it, enjoy your weekend

@lucasfernog
Copy link
Member

I started taking a look at it but got carried away to other things. I was trying to improve the return type but.. it's complicated

Copy link
Contributor

@Legend-Master Legend-Master left a comment

Choose a reason for hiding this comment

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

Desktop code looks good, tested on Windows

Comment on lines +96 to +125
export type MessageDialogButtonsYesNoCancel = {
/** The Yes button. */
yes: string
/** The No button. */
no: string
/** The Cancel button. */
cancel: string
}

/**
* The Ok and Cancel buttons of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOkCancel = {
/** The Ok button. */
ok: string
/** The Cancel button. */
cancel: string
}

/**
* The Ok button of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOk = {
/** The Ok button. */
ok: string
}
Copy link
Contributor

@Legend-Master Legend-Master Apr 19, 2025

Choose a reason for hiding this comment

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

Just a small suggestion, maybe we can change these to something like this, so typescript doesn't hint ok and yes at the same type

Suggested change
export type MessageDialogButtonsYesNoCancel = {
/** The Yes button. */
yes: string
/** The No button. */
no: string
/** The Cancel button. */
cancel: string
}
/**
* The Ok and Cancel buttons of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOkCancel = {
/** The Ok button. */
ok: string
/** The Cancel button. */
cancel: string
}
/**
* The Ok button of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOk = {
/** The Ok button. */
ok: string
}
export type MessageDialogButtonsYesNoCancel = {
/** The Yes button. */
yes: string
/** The No button. */
no: string
/** The Cancel button. */
cancel: string
}
/**
* The Ok and Cancel buttons of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOkCancel = {
/** The Ok button. */
yes: string
/** The Cancel button. */
cancel: string
}
/**
* The Ok button of a message dialog.
*
* @since 2.3.0
*/
export type MessageDialogButtonsOk = {
/** The Ok button. */
yes: string
}

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.

Allow for a third button in the MessageDialogBuilder
4 participants