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

Chore: Update Modals to SurfaceView #150

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

devanshkansagra
Copy link

@devanshkansagra devanshkansagra commented Jan 13, 2025

Description

Update RocketChat Apps-Engine to a newer compatible version and remove deprecated methods like ModalView to resolve issues with modals not functioning properly.

Closing issues

Tasks

  • Remove all possible deprecated methods of ModalView
  • Add new Updated Methods of SurfaceView

Video Proof

Screencast.from.16-01-25.09.10.41.PM.IST.webm

Note: The video will be updated once the other modals are updated as per the commit.

@devanshkansagra
Copy link
Author

devanshkansagra commented Jan 17, 2025

Hey @samad-yar-khan, I have found a small bug, in this bug there improper displaying of merge and approve button in pullDetails Modal. Meaning the merge and approve button was showing up even when the pr is merged or having conflicts, or even the user has no access of merging code to the repository, and pr is merged or closed

so in this fix I have added some conditions such that merge button should display if the user has merge access and that pr has no conflicts. in case of conflicts or if that user is not a maintainer then that button will not show up. the approve button will only show up when the pr is not merged

Video demo

2025-01-18.00-47-49.mp4

This similar functionality will also be obtained when executing this command /github username/reponame pulls <pr-number>

@devanshkansagra devanshkansagra marked this pull request as ready for review January 22, 2025 12:51
@devanshkansagra
Copy link
Author

devanshkansagra commented Jan 22, 2025

Hey @samad-yar-khan, this pull request is now ready for review. It includes a significant amount of work, so a thorough review might take some time. I’d be happy to assist with any clarifications or additional details if needed.

Creating a detailed video to showcase all the possible cases can take a long time, till then (if you are available and free) you can fetch this branch and deploy it to test the changes on your end and soon I will update the description video.

Thanks.

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.

1 participant