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

Orb of Rebirth should reload website #11114

Closed
Alys opened this issue Apr 13, 2019 · 17 comments · Fixed by #11125
Closed

Orb of Rebirth should reload website #11114

Alys opened this issue Apr 13, 2019 · 17 comments · Fixed by #11125

Comments

@Alys
Copy link
Contributor

Alys commented Apr 13, 2019

When you use the Orb of Rebirth, the website should reload itself as soon as the Rebirth has occurred so that all changes caused by Rebirth are seen immediately.

Currently, there's no visual change after Rebirth has happened so it looks like Rebirth hasn't worked. Worst of all, if the user was at or above level 100 and so had Rebirth available for free, it looks like it's still free so they try to use it again not realising they'll be charged gems.

I'm marking this as high priority since it's causing players to buy Rebirth multiple times, which wastes their gems.

@GiacomoLaw
Copy link
Contributor

GiacomoLaw commented Apr 16, 2019

Would adding location.reload(); to the below function work?

return [
{user, tasks},
i18n.t('rebirthComplete'),
];

@citrusella
Copy link
Contributor

citrusella commented Apr 19, 2019

For what it's worth, the modal you get when you click the achievement notification (which does pop up in the corner of the page when you rebirth) still properly reloads the site when closed.

(Would making the site reload without actioning that notification prevent it from ever being available to the player? Would returning that notification into a generates-a-modal-instead-of-a-notification kind be an alternative solution?

@HydeHunter2
Copy link
Contributor

I'd like to take this

@citrusella
Copy link
Contributor

citrusella commented Apr 22, 2019

Question I still feel merits some kind of response: Does the suggested solution completely remove the rebirth achievement modal by way of rendering it inaccessible (that is, is there no delay in reload that would allow the notification rebirth generates to be clicked)? (I might have a sentimental attachment to that modal because I like the information it tells me, i.e. the whole "congrats, this is rebirth x for you, etc. etc." but I feel this is a question worth asking.)

Would it be more prudent to fix the issue by way of making the modal a modal-modal (like it was at some point previously) and not a notification-modal?

@paglias
Copy link
Contributor

paglias commented Apr 23, 2019

@Tressley do you have any indication regarding @citrusella 's question on how you'd like this to be solved?

@HydeHunter2
Copy link
Contributor

If I understand correctly what you want, then the achievement "Began a new Adventure" will not be removed.

Here's how it will happen if this pull request is merged:

  1. You buy Orb of Rebirth.
  2. A notice "You bought the Orb of Rebirth" appears.
  3. The page automatically reloads.
  4. And only then the achievement "Began a new Adventure" appears.
  5. You can click on it to see the modal with congratulations.

@citrusella
Copy link
Contributor

Oh. I couldn't tell that from just reading the PR. Is this a case of my kindergarten-level understanding of Javascript again? (I might have also been remembering a past instance where the site instead of the modal initiating the reload resulted in the modal being unreadable--though in that instance it was a modal that popped up before the site reloaded, not a notification or a modal that appeared after.)

Just to make sure I understand correctly, you are referring to the "snackbar" (righthand side) notification for rebirth that opens the modal that includes the rebirthAchievement/rebirthAchivement100 text, right? (Saying "the achievement won't be removed" briefly had me thinking you thought I meant the actual achievement, like the one in the profile. XP)

@Tressley
Copy link
Collaborator

I think points 4 and 5 should be consolidated into one. I don't think we should be tucking achievements behind snack notifications.

@HydeHunter2
Copy link
Contributor

Okay, I'll fix it soon

@Tressley
Copy link
Collaborator

@HydeHunter2 -- With that being said, I think we need to give the achievement modal some attention because at the moment you have no idea what you've achieved. 😅

image

@citrusella
Copy link
Contributor

citrusella commented Apr 23, 2019

In that case, is it better to just un-snack the existing notification? Clicking close on it already initiates a reload. Or is changing the reload to before, showing it after, and un-snacking better?

Apparently the snacking came from the issue I linked in my last comment, but there's like four or five modals that have been put inside snackbars? (Or seven, apparently.) Looking at the chat on that PR actually appears that the "bug" may have been introduced then? It appears rebirth might reload the website without closing (or even opening!) the modal but not immediately.

@HydeHunter2
Copy link
Contributor

Hmm, this is the screenshot[you sent] of what achievement?

Just the achievement "Began a New Adventure" has a description. Or are you talking about something else?
Снимок экрана 2019-04-23 в 21 47 44
Github

@HydeHunter2
Copy link
Contributor

@Tressley please help me choose.
Do I have to restart page before or after the achievement modal appears?

Like:

Option 1:
1) Buy Orb
2) Restart page
3) Show achievement modal 

OR

Option 2:
1) Buy Orb
2) Immediately show achievement modal
3) Restart page after pressing "Huzzah"

@HydeHunter2
Copy link
Contributor

I consolidated points 4 and 5 into one. If necessary, I'll move the reloading page, wherever you want.

P.S. Thanks, @citrusella ! You were right. This issue introduced after this pull request. Knowing this helped me thanks to you :]

@Tressley
Copy link
Collaborator

@HydeHunter2 -- The modal screenshot I shared is the modal that is displayed when you click on the snack notification. This was during testing on a new account.

As for the refresh, I think your first option is my preference.

@HydeHunter2
Copy link
Contributor

Okay, thanks for answer!
I think we've discussed everything and a pull request's code can be reviewed.

@paglias
Copy link
Contributor

paglias commented Apr 25, 2019

thanks @HydeHunter2 ! Sorry for the delay but before the end of the week I'll be sure to give a look at your PRs

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

Successfully merging a pull request may close this issue.

6 participants