erobit / meteor-accounts-ui-bootstrap-dropdown

Meteor accounts-ui package updated to use twitter/bootstrap dropdown
37 stars 218 forks source link

reset password form #8

Open svasva opened 11 years ago

svasva commented 11 years ago

Hi there!

The package is awesome, though password reset form looks broken. Could you please fix it? Thanks!

erobit commented 11 years ago

Hi erundook,

Can you please provide more detail? I don't have any active meteor projects on the go to test with, so a brief description of the problem you are encountering will save me some time.

svasva commented 11 years ago

just create an empty project, add your package to it, then register a user, log out and try forgot password link. the reset password email links to a broken page :(

mikpanko commented 11 years ago

Hi erundook,

Is this stackoverflow answer what you are looking for? If so, it seems that this is not an issue of this project and needs to be addressed within each application separately.

svasva commented 11 years ago

No. The issue is that the reset password form is broken on that page.

svasva commented 11 years ago

This is how it looks like: 2013-07-01 3 55 52

Feel free to sign up at https://wallet.epools.org and try to use the 'forgot password' feature.

mikpanko commented 11 years ago

I think I fixed the issue you are talking about. Here is what I get:

screen shot 2013-07-01 at 12 14 03 am

Here is the repo with the code. I am relatively new to JS and collaborative coding via Github, so it might not be in the best shape + I have fixed only the resetPassword template out of several. This is why I don't ask for a pull request on the main repo yet. Let me know what you think.

AleksandarFaraj commented 11 years ago

mikpanko I did try your fix, it did not work. Could you explain why we are getting these errors? I am not a fan of client side javascript/css/html hence why i use bootstrap! :)

Edit: It works. somehow I messed something up.

mikpanko commented 11 years ago

Aleksandar, glad to hear it worked. These errors happened because elements in "login_buttons_dialogs.html" had wrong classes and ids (probably, from the original package "accounts-ui"). They needed to be changed to correspond to bootstrap classes. In a similar way, "login_buttons_dialogs.js" needed to be modified to handle template events correctly. I fixed just one template ("_resetPasswordDialog") from the file. Several others still won't display nicely yet.

AleksandarFaraj commented 11 years ago

would it be to much to ask to fix the enroll email?

erobit commented 11 years ago

It wouldn't be too much to ask at all. I will look into some of the fixes this evening.

Sent from my iPhone

On Jul 4, 2013, at 7:18 PM, Aleksandar-Novakovic notifications@github.com wrote:

would it be to much to ask to fix the enroll email?

— Reply to this email directly or view it on GitHub.

AleksandarFaraj commented 11 years ago

you are fast! Thanks.

bradsimantel commented 11 years ago

I'm seeing this same issue. It looks like the password reset modal is just using non-Boostrap classes, so @mikpanko was on the right track. It looks like his fork is incorporating some other changes as well, though?

mikpanko commented 11 years ago

On top of changing CSS classes to Bootstrap style in "login_buttons_dialogs.html" I added dynamic modal behavior which required JS changes to "login_buttons_dialogs.js". Changes to "login_buttons_dropdown.js" do not do anything as new lines are all commented out (I should not have had them). However, all of these changes are obsolete now as @erobit has made more pushes to his original branch without even acknowledging my pull request. As I can tell, his new changes do not address the modal problem yet.

erobit commented 11 years ago

You are correct mikpanko. I apologize for not merging your contribution into mainline. I wanted to make sure I had feature parity with the latest accounts-ui-unstyled before merging in. Of course this means it's a bit more complicated to get your changes in. I am willing to spend some time to have your changes integrated so that we can close this issue. Maybe we can collaborate over the coming days to finalize the changes?

mikpanko commented 11 years ago

Sure, @erobit. Let me know how I can help. Thank you for working on this project.

apukhraj commented 11 years ago

@erobit , any chance this was merged ? i just upgraded to the latest version ( with the latest version of meteor, 0.6.5.1) and i am still seeing the same issue. And, Yes, thank you for this project.