Skip to content

Conversation

jd-alexander
Copy link
Contributor

@jd-alexander jd-alexander commented Aug 20, 2019

Fixes #8002

Background

The dismissAfterConfirm flag will determine if the dialog will be dismissed after the confirm action is clicked. Its default behavior is to dismiss after confirm since the existing implementations utilize this.

This change is being made because the username changer will need to show a confirmation & progress dialog and the result of the associated operations will determine if the dialog should be dismissed or not.

Main Changes

The builder now has a parameter for the flag, and this is how the functionality is initiated.

 new FullScreenDialogFragment.Builder(getContext())
                .setTitle(R.string.username_changer_title)
                .setAction(R.string.username_changer_action)
                .setOnConfirmListener(this)
                .setOnDismissListener(this)
                .setDismissAferConfirm(false) // this is the flag.
                .setContent(UsernameChangerFullScreenDialogFragment.class, bundle)
                .build(); 

Testing

  1. Use the same methods described in the WP Android block of the abstract username fragment PR description to access the suggestions screen easily.

  2. Simply modify the builder that's used to launch the dialog in the SignUpEpilogueFragment with the dismissAfterConfirm flag similar to the example above.

The result of this will be that clicking the Save action will do nothing.
Notice in the example below that the ripple effect signifying the click is shown on the Save action but no dismissal occurs.

Left - Default behaviour. Right - Dismiss disabled.
( Sorry for the differences in screen size. Utilized two different devices when testing 😅)

left ezgif com-video-to-gif (4)

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

This flag will determine if dialog will be dismissed after confirm is clicked. It's default behavior is to dismiss after confirm since the existing implementations utilize this.
@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is missing at least one label.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@maxme maxme changed the base branch from develop to issue/8002-change-username August 21, 2019 11:28
@maxme
Copy link
Contributor

maxme commented Aug 21, 2019

Note: I switched the base branch to issue/8002-change-username that we'll use for this and for followup PR related to that feature.

Copy link
Contributor

@maxme maxme left a comment

Choose a reason for hiding this comment

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

Looks good to me, first step for adding confirmation or progress.

@maxme
Copy link
Contributor

maxme commented Aug 21, 2019

:shipit: (Merging it into the feature branch).

@maxme maxme merged commit ef150d1 into wordpress-mobile:issue/8002-change-username Aug 21, 2019
@jd-alexander jd-alexander deleted the fullscreen_dialog_confirm_fix branch August 21, 2019 23:37
@jd-alexander
Copy link
Contributor Author

Note: I switched the base branch to issue/8002-change-username that we'll use for this and for followup PR related to that feature.

Sounds good. I like this kind of branching strategy.

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.

2 participants