Skip to content

Conversation

@Spiral-Memory
Copy link
Contributor

Added a overflow button to go to "Create subpage or record modal" from "Create database" modal as there was no provision for that.

Issue(s)

Closes #47

Acceptance Criteria fulfillment

  • Found out how to create an overflow menu using OverflowMenuComponent.
  • Investigated the process for adding new menu text and an Action ID, changed the enum file and createDatabaseModal.ts accordingly.
  • Implemented the overflow menu and handled the case in the handler when this "Create Subpage" menu is clicked.

Proposed changes (including videos or screenshots)

2024-01-29.12-08-20.mp4

Further comments

@Spiral-Memory Spiral-Memory force-pushed the feat/added-overflow-btn-change-subpage branch from 1b3ed6c to 9312d03 Compare January 29, 2024 20:37
@Spiral-Memory
Copy link
Contributor Author

@Nabhag8848 , as per your suggestion, I have added the "Change Workspace Overflow" button to the Create Database Modal, and the PR is now ready for review. I am also attaching the latest demo video for verification.

2024-01-30.02-08-12.mp4

@Nabhag8848 Nabhag8848 self-requested a review February 7, 2024 06:27
Copy link
Collaborator

@Nabhag8848 Nabhag8848 left a comment

Choose a reason for hiding this comment

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

Hey ! @Spiral-Memory Added few comments and also can we have a functionality When switching to change workspace inside create database modal once the workspace has been changed can we move back and open up create database modal ? this way users can avoid writing command for opening the same modal of create database again.
We need to Make sure this happens only when change workspace is open from create database Modal and not from regular /notion workspace.

  • @Sayan4444 has been working on similar thing #49 . check channel to see how it can be implemented you can collab if both the party agrees.

@Spiral-Memory
Copy link
Contributor Author

Spiral-Memory commented Feb 7, 2024

Hey ! @Spiral-Memory Added few comments and also can we have a functionality When switching to change workspace inside create database modal once the workspace has been changed can we move back and open up create database modal ? this way users can avoid writing command for opening the same modal of create database again. We need to Make sure this happens only when change workspace is open from create database Modal and not from regular /notion workspace.

Sure, @Nabhag8848 . I will review it and consider how it can be implemented. I'll also discuss it with Sayan on that PR to see if both of us can collaborate. I am also going through it, I will let you know once i figure out a way to do, i will then push the changes here.

@Spiral-Memory
Copy link
Contributor Author

Spiral-Memory commented Feb 7, 2024

Hey, I tried implementing it. I am passing a state to keep track of whether it has been opened using the overflow button, and it is working. However, the problem I am facing is that as soon as the modal opens, it is closing automatically. I'm not sure why this is happening. Trying to debug. I will let you know about the progress. If you know why this might be happening, please let me know.

I am attaching a video for the same. In this video, you will be able to see that when I try to change the workspace after opening it through the Create Database model and submit it, the system moves me back to the Create Database model but only for a second it stays and then closes (You can play it slowly or pause around 0:17 timestamp, to see the effect). However, when I use /notion workspace and then make the change, there is no such issue, so what we wanted is implemented. The problem is that, in the first case, it is unable to hold its place and closes abruptly.

I feel that for Submit buttons which is passed as submit argument to openSurfaceView which executes ExecuteViewSubmitHandler once the button is clicked, clearing all the modals is a default behavior. I am not sure, but that might be the reason. Do you know of any way I can stop this behavior?

2024-02-07.23-20-12.mp4

@Nabhag8848
Copy link
Collaborator

Nabhag8848 commented Feb 8, 2024

I feel that for Submit buttons which is passed as submit argument to openSurfaceView which executes ExecuteViewSubmitHandler once the button is clicked, clearing all the modals is a default behavior. I am not sure, but that might be the reason. Do you know of any way I can stop this behavior?

  • @Spiral-Memory @Sayan4444 Oops Yep ! This looks fine and its default behaviour and as far as i remember submit handler closes all the modal. Can any one of you reach out to someone to ask if there is workaround for same that would be good exercise to know more about mindset around this behaviour of apps engine ?

@Spiral-Memory
Copy link
Contributor Author

Sure @Nabhag8848 , Will try asking more about it in the community. Also I'm going through the RC-engine repo to see if anything can be done.

@Nabhag8848
Copy link
Collaborator

Nabhag8848 commented Feb 13, 2024

@Spiral-Memory with the old implementation of uikit like in github app reopening of the modals can be achieved on Submit Action, Right ?

@Spiral-Memory
Copy link
Contributor Author

@Spiral-Memory with the old implementation of uikit like in github app reopening of the modals can be achieved on Submit Action, Right ?

Yes, @Nabhag8848 ,there is a function called openModalViewResponse that I have seen in the GitHub app code. With that, I feel it can be achieved. The function accepts IUKitModalViewParam and not the surface one.

@Nabhag8848
Copy link
Collaborator

Nabhag8848 commented Feb 13, 2024

@Spiral-Memory with the old implementation of uikit like in github app reopening of the modals can be achieved on Submit Action, Right ?

Yes, @Nabhag8848 ,there is a function called openModalViewResponse that I have seen in the GitHub app code. With that, I feel it can be achieved. The function accepts IUKitModalViewParam and not the surface one.

@Spiral-Memory if thats the case, Ask and Tag Devanshu in Apps-Development Was there changes in new UI Kit on how modals are handled ? cause this potentially may not be not expected behaviour, if same can be achieved with old ui kit implementation. (Submit Action) + verify it yourself by spinning new app and implementing this with old UI before doing that.

Note that: Button as a Action or within Section in Modals can achieve this already but we are talking about Submit Action

@Spiral-Memory
Copy link
Contributor Author

@Spiral-Memory with the old implementation of uikit like in github app reopening of the modals can be achieved on Submit Action, Right ?

Yes, @Nabhag8848 ,there is a function called openModalViewResponse that I have seen in the GitHub app code. With that, I feel it can be achieved. The function accepts IUKitModalViewParam and not the surface one.

@Spiral-Memory if thats the case, Ask and Tag Devanshu in Apps-Development Was there changes in new UI Kit on how modals are handled ? cause this potentially may not be not expected behaviour, if same can be achieved with old ui kit implementation. (Submit Action) + verify it yourself by spinning new app and implementing this with old UI before doing that.

Note that: Button as a Action or within Section in Modals can achieve this already but we are talking about Submit Action

Sure, @Nabhag8848 , I will try creating a new app and testing all possible behaviors of both older and newer implementations. Also, I will tag Devanshu and ask as well.

And yeah, I know that using a button as an action or within a section can be achieved; I have tried that, and it was working. The issue arises with "Submit" as an action. It abruptly closes all modals, even though the other modal opens for a second.

@Spiral-Memory
Copy link
Contributor Author

Spiral-Memory commented Feb 13, 2024

Also, @Nabhag8848 , is there documentation that clearly states all such behaviors or how it works? Currently, the website has very basic documentation, and the list of functions on another website doesn't explain the functionality of everything. Is there proper documentation available to help understand RC app development in a much better way? The current way of understanding is mainly to look at the existing rc app and replicate the behaviour.

@Sayan4444
Copy link
Contributor

Yes there is all the function definitions are found here @Spiral-Memory
https://rocketchat.github.io/Rocket.Chat.Apps-engine/interfaces/accessors_IMessageExtender.IMessageExtender.html#addAttachment

@Spiral-Memory
Copy link
Contributor Author

Spiral-Memory commented Feb 13, 2024

Yes there is all the function definitions are found here @Spiral-Memory
https://rocketchat.github.io/Rocket.Chat.Apps-engine/interfaces/accessors_IMessageExtender.IMessageExtender.html#addAttachment

I'm aware of this, @Sayan4444 , but it doesn't give enough examples to understand how everything works. I'm looking for something that provides more information, it is more like an index of functions with very brief descriptions.

@Spiral-Memory
Copy link
Contributor Author

Hey @Nabhag8848 , just to update you, I've contacted Devanshu. He mentioned he will look into the issue, determine why we're experiencing unexpected behavior, and will let us know.

@Nabhag8848
Copy link
Collaborator

Hey @Nabhag8848 , just to update you, I've contacted Devanshu. He mentioned he will look into the issue, determine why we're experiencing unexpected behavior, and will let us know.

  • Awesome ! maybe if once your exams are finished you can try to find out the issue yourself.

@Spiral-Memory
Copy link
Contributor Author

Hey @Nabhag8848 , just to update you, I've contacted Devanshu. He mentioned he will look into the issue, determine why we're experiencing unexpected behavior, and will let us know.

  • Awesome ! maybe if once your exams are finished you can try to find out the issue yourself.

Yes @Nabhag8848, For sure !

@Spiral-Memory
Copy link
Contributor Author

Hey @Nabhag8848 , I went through the RC engine repo and tried building the RC app, but I still can't find the problem. So, for now, I feel we should stick to halting the "going back" feature and focus on other issues. Also, please review this and other PRs whenever you are free. If I find anything related to this particular feature, I'll let you know sir.

@Sayan4444 , did you find a fix for it?

@Sayan4444
Copy link
Contributor

@Spiral-Memory No I tried a lot

@Spiral-Memory
Copy link
Contributor Author

@Spiral-Memory No I tried a lot

Same 🥲

@Nabhag8848
Copy link
Collaborator

Hey @Nabhag8848 , I went through the RC engine repo and tried building the RC app, but I still can't find the problem. So, for now, I feel we should stick to halting the "going back" feature and focus on other issues. Also, please review this and other PRs whenever you are free. If I find anything related to this particular feature, I'll let you know sir.

@Sayan4444 , did you find a fix for it?

Thanks @Spiral-Memory @Sayan4444 thats sounds cool to me for now. Will merge this and review all leftover PRs. just stuck with some of my deadlines. Will do it today at night🙏🏻

@Spiral-Memory
Copy link
Contributor Author

Hey @Nabhag8848 , I went through the RC engine repo and tried building the RC app, but I still can't find the problem. So, for now, I feel we should stick to halting the "going back" feature and focus on other issues. Also, please review this and other PRs whenever you are free. If I find anything related to this particular feature, I'll let you know sir.

@Sayan4444 , did you find a fix for it?

Thanks @Spiral-Memory @Sayan4444 thats sounds cool to me for now. Will merge this and review all leftover PRs. just stuck with some of my deadlines. Will do it today at night🙏🏻

Sure @Nabhag8848 Sir. Thanks a lot 😃

@Sayan4444
Copy link
Contributor

Hey @Nabhag8848 , I went through the RC engine repo and tried building the RC app, but I still can't find the problem. So, for now, I feel we should stick to halting the "going back" feature and focus on other issues. Also, please review this and other PRs whenever you are free. If I find anything related to this particular feature, I'll let you know sir.
@Sayan4444 , did you find a fix for it?

Thanks @Spiral-Memory @Sayan4444 thats sounds cool to me for now. Will merge this and review all leftover PRs. just stuck with some of my deadlines. Will do it today at night🙏🏻

Thanks bhaiya for giving me the chance to help

@Nabhag8848 Nabhag8848 changed the title feat: added a overflow button to go to create subpage modal from create database modal [FEAT] Overflow Menu in CreateDatabase Modal Includes Change to Workspace and CreatePageOrRecord Modal Feb 29, 2024
"Create Page" text added
@Nabhag8848 Nabhag8848 added the enhancement New feature or request label Feb 29, 2024
@Nabhag8848 Nabhag8848 merged commit 4641ad8 into RocketChat:main Feb 29, 2024
@Nabhag8848
Copy link
Collaborator

Congrats @Spiral-Memory ! Hope you loved going depth on researching about issues in apps engine.

  • The amount of satisfaction you get when PR getting merged after so much work is unmatchable, it reminds me of my pre gsoc season.
  • Also Great Collab with @Sayan4444 🚀

@Spiral-Memory
Copy link
Contributor Author

Congrats @Spiral-Memory ! Hope you loved going depth on researching about issues in apps engine.

  • The amount of satisfaction you get when PR getting merged after so much work is unmatchable, it reminds me of my pre gsoc season.
  • Also Great Collab with @Sayan4444 🚀

No doubt, @Nabhag8848 , it was a wonderful experience going through the engine repo and trying to build the app from scratch. It helped me learn as well. Yes, I can resonate with the feelings you had during your time at GSoC. Also, thank you so much for the appreciation and your guidance. I will continue to look into this feature, and if I see any improvement, I will let you know as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

[Feat]: Adding an overflow menu to navigate to the "Create Subpage" modal from "Create Database" modal.

3 participants