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

Change progress dialog cancel button visibility state #2929

Merged

Conversation

avazirna
Copy link
Contributor

@avazirna avazirna commented Dec 19, 2024

Summary

This PR reverts a change that appears to have been made by mistake during the UI revamping work, which caused a Stop button to be visible during app installations.
Ticket: https://dimagi.atlassian.net/browse/QA-7351

PR Checklist

  • If I think the PR is high risk, "High Risk" label is set
  • I have confidence that this PR will not introduce a regression for the reasons below
  • Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly
  • Does the PR introduce any major changes worth communicating ? If yes, "Release Note" label is set and a "Release Note" is specified in PR description.

@avazirna avazirna requested a review from pm-dimagi December 19, 2024 22:39
@avazirna avazirna marked this pull request as ready for review December 19, 2024 22:39
@avazirna avazirna changed the base branch from master to commcare_2.55 December 19, 2024 22:40
Copy link

coderabbitai bot commented Dec 19, 2024

📝 Walkthrough

Walkthrough

The pull request introduces a modification to the progress_dialog_cancel_button.xml layout file, specifically changing the visibility attribute of a MaterialButton. The button's visibility is altered from "visible" to "gone", which means the button will no longer be displayed or occupy space in the user interface when the progress dialog is rendered. This change effectively removes the cancel button from the dialog's layout, potentially changing the user interaction pattern for the specific dialog where this layout is used.

The modification is a straightforward UI adjustment that impacts the visual presentation and interaction possibilities of the progress dialog by removing the cancel button entirely, rather than just hiding it while maintaining its layout space.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4238469 and d00350d.

📒 Files selected for processing (1)
  • app/res/layout/progress_dialog_cancel_button.xml (1 hunks)
🔇 Additional comments (2)
app/res/layout/progress_dialog_cancel_button.xml (2)

17-17: LGTM! Visibility change aligns with PR objective.

The change from visible to gone correctly removes the Stop button during installation, preventing potential issues from interrupting the installation process.


17-17: Verify button visibility states during installation flow.

Since this is a high-risk change affecting the installation process, please ensure:

  1. The button remains hidden throughout all installation states
  2. There are no edge cases where cancellation should still be allowed

Let's verify the usage of this layout in the installation flow:

✅ Verification successful

Based on the search results, I can see that this layout file progress_dialog_cancel_button.xml is included in two other layout files:

  1. progress_dialog_determinate.xml
  2. progress_dialog_indeterminate.xml

Let me search for more specific usage of these progress dialogs in the installation flow.


Let me search for more specific usage of CustomProgressDialog in installation-related code to understand how the cancel button visibility is managed.


Based on the search results, I can now provide a definitive response about the cancel button visibility change:

Verified: Cancel button visibility is correctly set to GONE during installation

The code review shows that:

  1. The installation progress dialogs (both determinate and indeterminate) are explicitly set as non-cancellable in CommCareSetupActivity.java with dialog.setCancelable(false)
  2. The progress dialog layouts include this cancel button layout with visibility GONE
  3. There are no instances where the cancel button visibility is dynamically changed during installation

This confirms that:

  • The button remains hidden throughout all installation states
  • Installation process is designed to be non-cancellable by setting setCancelable(false) at the dialog level
  • The visibility change aligns with the intended non-cancellable behavior of the installation process
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for references to this layout file and its usage in installation-related code
rg -A 5 "progress_dialog_cancel_button" 

# Look for any dynamic visibility changes to this button
ast-grep --pattern 'setVisibility($_)' 

# Check for installation-related classes/activities that might use this dialog
rg -A 5 -i 'install|setup|progress.*dialog'

Length of output: 73282


Script:

#!/bin/bash
# Search for references to these progress dialog layouts in installation-related code
rg -A 5 "progress_dialog_(determinate|indeterminate)" app/src/

# Look for any code that manages cancel button visibility during installation
ast-grep --pattern 'setVisibility($vis)' app/src/org/commcare/activities/

Length of output: 1188


Script:

#!/bin/bash
# Search for installation-related code using CustomProgressDialog
rg -A 10 "CustomProgressDialog" app/src/org/commcare/activities/CommCareSetupActivity.java app/src/org/commcare/activities/InstallArchiveActivity.java

# Look for any code that manages dialog cancellation during installation
rg -A 5 "setCancelable|setOnCancelListener" app/src/org/commcare/activities/

Length of output: 10487


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@avazirna avazirna merged commit 9a14d00 into commcare_2.55 Dec 19, 2024
5 of 6 checks passed
@avazirna avazirna deleted the fix-progress-dialog-cancel-button-visibility branch December 19, 2024 23:36
@coderabbitai coderabbitai bot mentioned this pull request Dec 20, 2024
4 tasks
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