-
Notifications
You must be signed in to change notification settings - Fork 49
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
Remove mod button in advance option added #105
base: dev
Are you sure you want to change the base?
Remove mod button in advance option added #105
Conversation
@AvinashAgarwal14 A rather useful feature here could be to allow the user to access the add mod button (the '+' button) only once all the above moderator fields have been filled. This would not only eradicate the need of the delete mod button, but would also make the instance creation process smoother :) |
@AvinashAgarwal14 and @sachin235 I think a mix of both of your suggestions is right here. @AvinashAgarwal14 having that (-) button or an (x) isn't a bad idea on this page. However it might want to be to the right of each line allowing you to remove the line if you change your mind before submitting. Coupled with logic about the input field would be great. @sachin235 You are right that there are logic issues here. Making sure there is input in the field before adding another is a good catch. Another logic issue here is that you can add the Admin as a mod to the instance. You can also add the same mod more then once. Some logic to catch who is already added to a session with roll would be good here as well. As a note for future PRs all of those are all separate issues and probably shouldn't be lumped too much into the same PR. |
@okpop Thank you for reviewing the PR, I will make the changes with respect to the (-) button in this PR. Regarding the logic issues, I will try and find a solution for the same and open separate PRs. |
119ab36
to
e140572
Compare
@okpop I have introduced the changes, please review them. Thanks :) |
@okpop I have already discussed and included these logic issues in the GSoC proposal, and also proposed their implementation within the timeline. |
This looks great in terms of UI. Incorporating the logic about input field would make it more appealing. |
Thanks. In the last commit, I have included a basic input logic of not letting the user create a new input field if any one of the previous fields is empty. |
18c4e1e
to
4167cce
Compare
Fixes: #104
Before:
After: