Remember user inputs after failed registration #4982 #4983
Conversation
access user input in case registering to a server fails and the user is sent back to the registration input dialogue again. This saves the user the effort of re-typing everything, when presumably only one thing is incorrect Adds the feature requested from issue Cockatrice#4982 See Cockatrice#4983 for more information
candyapplecorn
force-pushed
the
remember-user-inputs-after-failed-registration
branch
from
cc72cf8
to
ddf8487
Compare
cockatrice/src/dlg_register.cpp
Outdated
| playernameEdit->setMaxLength(MAX_NAME_LENGTH); | ||
| playernameLabel->setBuddy(playernameEdit); | ||
|
|
||
| passwordLabel = new QLabel(tr("P&assword:")); | ||
| passwordEdit = new QLineEdit(); | ||
| passwordEdit = new QLineEdit(abstractClient->password); |
There was a problem hiding this comment.
why not instead of all this messing around with accessing the client's private fields just pass the needed information to the dialog's constructor?
There was a problem hiding this comment.
why not instead of all this messing around with accessing the client's private fields just pass the needed information to the dialog's constructor?
sure, that would work. Pretty sure I wanted to do it that way as well, cannot remember exactly why I decided to stop and try a different way. I believe we would need to modify the remote client (or the abstract client it inherits from) so that the window_main file could access those values.
So I think in the end we would still need to modify some class to give permission to another class to access its data
A better solution still would be to keep the dialogue open and not destroy it until the user has exited out of the flow by either canceling or quitting the program or successfully registering. Somewhere, there are some signals that cause the dialogue register to close. There's gotta be a failure one and a success one and maybe one or two others for errors or who knows what. We would need to find all of those slots that are listening for those signals and modify them to not close the dialogue DlgRegister if the user encounters a registration error. I also think just hitting the submit button on the registration dialogue closes the registration dialogue, so that one would need to be modified as well. This would definitely be the best approach if I could pull it off though because then we wouldn't need to modify any of the accessibility of the class attributes.
Because by simply having the same dialogue stay open, it would remember what its values are, since it never would've closed in the first place.
There was a problem hiding this comment.
I've been meaning to do that yet still haven't found the time to sit down and get it all sorted, it needs a lot of signals and stuff moved #4586
my comment here is just regarding the arguments in the constructor of the dialog, no changes to the header of the client will be required, just pass the values when the dialog is instantiated.
There was a problem hiding this comment.
Sounds like an easy change to make then @ebbit1q . RAII is good. I hope to get around to it not too long from now. Thanks for your advice
There was a problem hiding this comment.
I've changed the constructor to accept arguments: https://github.com/Cockatrice/Cockatrice/pull/4983/files#diff-1b35d27e75120bfb88322ce520f3c057ab93b345875738aba24e04a8596b3285R16
cockatrice/src/window_main.cpp
Outdated
| QString password = client->password; | ||
| QString email = client->email; | ||
| QString country = client->country; | ||
| QString realname = client->realName; |
There was a problem hiding this comment.
@ebbit1q now MainWindow is the friend class instead of DlgRegister, and these variables are passed to the DlgRegister constructor
|
I've made a few changes to your pr:
saving the values in the main window isn't better or anything, I can also see some more accessors being added to the client for this part. our current style is to not use direct access to properties, instead being more similar to qt's style and making accessors for properties. for me placing the values in the main window for now does make it simpler to work with #4586 as it does less changes. |
|
@candyapplecorn Did you have a chance to review the last changes? |
Related Ticket(s)
Short roundup of the initial problem
Registering, when being rejected, would reset all of the input fields. That makes the user type them all over again, when all they should need to do is type the one that was incorrect.
What will change with this Pull Request?
More specifically, I decided to make
DlgRegistera friend class ofAbstractClient. This way, the registration dialogue can access the protected members ofAbstractClient.This solution keeps the
AbstractClient's protected information safe, as it is only allowing the registration dialogue (DlgRegister) to access this information. All other parts of the code base that could not access this protected information inside ofAbstractClientstill can't access it.Screenshots
BEFORE:
original_behavior.mov
AFTER:
Notice that the user's information remains in the input forms of
DlgRegisterScreen.Recording.2024-01-12.at.9.10.56.PM.mov