This repository was archived by the owner on Nov 28, 2022. It is now read-only.
Merged
Conversation
- Adds "impersonate" button which would be triggering a popup window - The html structure was changed to resemble closer the structure on other views (e.g. tags)
- This function doesn't fit to it's own model just yet and is always used in context of the member. - Also the URL strcucture is different to standard resources, so having an adapter made constructing custom URL a lot simpler
naz
force-pushed
the
members-login-link
branch
from
c2fab0e to
db3e089
Compare
kevinansfield
suggested changes
Feb 25, 2020
Member
kevinansfield
left a comment
kevinansfield
left a comment
There was a problem hiding this comment.
I wouldn't use a non-model-associated adapter here. It would be a lot cleaner to put the ajax call directly into the Member.fetchSigninUrl method.
Contributor
Author
|
@kevinansfield refactored the adapter call to a straight ajax in the member model. Will be merging this later today unless there are any other objections 😉 |
naz
force-pushed
the
members-login-link
branch
from
db3e089 to
5028737
Compare
kevinansfield
approved these changes
Feb 26, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refs TryGhost/Ghost#11607
@kevinansfield single thing that I have in question for this PR is the use of adapter on member model
2b98a3d, instead of a separate model for
signin_urlresource. Do you think this is clean approach or should be done differently?