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

feature request: Duplicate Saved capability sets name #1196

Closed
Dor-bl opened this issue Nov 13, 2023 · 3 comments · Fixed by #1483
Closed

feature request: Duplicate Saved capability sets name #1196

Dor-bl opened this issue Nov 13, 2023 · 3 comments · Fixed by #1483
Labels
enhancement New feature or request

Comments

@Dor-bl
Copy link

Dor-bl commented Nov 13, 2023

Current Behavior

Currently when saving caps set with same name no message is shown
image

Suggested Solution

I believe we should avoid having the same capability set names

Additional Information

No response

@Dor-bl Dor-bl added the enhancement New feature or request label Nov 13, 2023
@ath0mas
Copy link
Contributor

ath0mas commented Nov 27, 2023

@eglitise I looked into it yesterday but not used to React and antd.
So finding duplicate name and throw Error in async saveSession is pretty easy, but Modal is still closed. Any suggestions on how to report this error, inside form, live disabling OK button, or onOK click but with error dialog and not closing the Modal, ... ?

@eglitise
Copy link
Collaborator

@ath0mas thank you for trying to help out 🙏
I would go for the approach that shows an error message inside the modal, by modifying the onOk handler (onSaveAsOk in CapabilityEditor). My suggestion would be to grab the savedSessions value from props, compare the saved names against saveAsText, and then save the result boolean in some local variable. Finally add an antd Alert whose visibility then depends on this boolean (you can check how this is used in e.g. the Session component file with isAttaching).
Although, this would only cover the new saved session case, but not the renaming of an existing session... not sure about that one yet.

@sudharsan-selvaraj
Copy link
Contributor

@eglitise i have created a PR with the changes. Looking forward for your review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants