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

Fix Go To Dash 'View' button #1024

Merged
merged 1 commit into from May 8, 2016

Conversation

Projects
None yet
4 participants
@yanyi

yanyi commented Apr 26, 2016

Fixed Go To Dash 'View' button previously messing up the iframe. Fixes #1014.

Used the old jQuery style of setting the Dashboard text as empty as I could not find documentation on how to use XKit.iframe.hide_button();

@nightpool

This comment has been minimized.

Show comment
Hide comment
@nightpool

nightpool Apr 26, 2016

Member

Hey! This looks great, thank you! Unfortunately, we should do something a little more to fix the hide_button function in xkit_patches. Can you take care of that?

Thanks!

Member

nightpool commented Apr 26, 2016

Hey! This looks great, thank you! Unfortunately, we should do something a little more to fix the hide_button function in xkit_patches. Can you take care of that?

Thanks!

@nightpool

This comment has been minimized.

Show comment
Hide comment
@nightpool

nightpool Apr 27, 2016

Member

Also, you need to rebase or squash any merge commits, thanks!

Member

nightpool commented Apr 27, 2016

Also, you need to rebase or squash any merge commits, thanks!

@yanyi

This comment has been minimized.

Show comment
Hide comment
@yanyi

yanyi May 2, 2016

Hey @nightpool, sorry, I was actually waiting for you to reply on my code changes. Didn't know if I was doing it right since it's my first time making pull request. Been checking this PR daily waiting for a reply actually, haha. But now that 'This branch is out-of-date with the base branch' I am not sure what steps to take.

yanyi commented May 2, 2016

Hey @nightpool, sorry, I was actually waiting for you to reply on my code changes. Didn't know if I was doing it right since it's my first time making pull request. Been checking this PR daily waiting for a reply actually, haha. But now that 'This branch is out-of-date with the base branch' I am not sure what steps to take.

@nightpool

This comment has been minimized.

Show comment
Hide comment
@nightpool

nightpool May 2, 2016

Member

Currently your PR has 2 junk merge/revert commits and a change to the unrelated "bookmarker" extension, and it needs to be rebased against master so I can merge it. You can read more about rebasing here: https://www.atlassian.com/git/tutorials/rewriting-history/

I'll take a look at both this or #1038 when I get a chance and integrate something that makes sense. Thanks for the contribution!

Member

nightpool commented May 2, 2016

Currently your PR has 2 junk merge/revert commits and a change to the unrelated "bookmarker" extension, and it needs to be rebased against master so I can merge it. You can read more about rebasing here: https://www.atlassian.com/git/tutorials/rewriting-history/

I'll take a look at both this or #1038 when I get a chance and integrate something that makes sense. Thanks for the contribution!

Yan Yi Goh
Fix XKit.iframe.hide_button() for Go To Dash
- Amended previously modified Go To Dash
- Modified XKit.iframe.hide_button() to hide Tumblr's .button-label
class
- Fixes #1014.
@yanyi

This comment has been minimized.

Show comment
Hide comment
@yanyi

yanyi May 3, 2016

@nightpool I read through the tutorial and went ahead with an interactive rebase that drop the previously unneeded commits to clean it up. Then I rebased it against the XKit's master. Could you please advise me if I have done it correctly?

yanyi commented May 3, 2016

@nightpool I read through the tutorial and went ahead with an interactive rebase that drop the previously unneeded commits to clean it up. Then I rebased it against the XKit's master. Could you please advise me if I have done it correctly?

@nightpool

This comment has been minimized.

Show comment
Hide comment
@nightpool

nightpool May 3, 2016

Member

Looks great. I don't have time to test these two until tomorrow or tonight,
but thanks for the quick response!

(also, feel free to drop by our gitter chat
https://gitter.im/new-xkit/xkit sometime and hang out and talk about dev
stuff!)

On Mon, May 2, 2016 at 10:19 PM Yan Yi Goh notifications@github.com wrote:

@nightpool https://github.com/nightpool I read through the tutorial and
went ahead with an interactive rebase that drop the previously unneeded
commits to clean it up. Then I rebased it against the XKit's master. Could
you please advise me if I have done it correctly?


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#1024 (comment)

Member

nightpool commented May 3, 2016

Looks great. I don't have time to test these two until tomorrow or tonight,
but thanks for the quick response!

(also, feel free to drop by our gitter chat
https://gitter.im/new-xkit/xkit sometime and hang out and talk about dev
stuff!)

On Mon, May 2, 2016 at 10:19 PM Yan Yi Goh notifications@github.com wrote:

@nightpool https://github.com/nightpool I read through the tutorial and
went ahead with an interactive rebase that drop the previously unneeded
commits to clean it up. Then I rebased it against the XKit's master. Could
you please advise me if I have done it correctly?


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#1024 (comment)

@yanyi

This comment has been minimized.

Show comment
Hide comment
@yanyi

yanyi May 4, 2016

Alright, sure thing. Would love to learn more! Thank you!

yanyi commented May 4, 2016

Alright, sure thing. Would love to learn more! Thank you!

@nightpool

This comment has been minimized.

Show comment
Hide comment
@nightpool

nightpool May 8, 2016

Member

Looks good to me @homu r+

Member

nightpool commented May 8, 2016

Looks good to me @homu r+

@homu

This comment has been minimized.

Show comment
Hide comment
@homu

homu May 8, 2016

📌 Commit f146230 has been approved by nightpool

homu commented May 8, 2016

📌 Commit f146230 has been approved by nightpool

homu added a commit that referenced this pull request May 8, 2016

Auto merge of #1024 - yanyi:master, r=nightpool
Fix Go To Dash 'View' button

Fixed Go To Dash 'View' button previously messing up the iframe. Fixes #1014.

Used the old jQuery style of setting the Dashboard text as empty as I could not find documentation on how to use `XKit.iframe.hide_button();`
@homu

This comment has been minimized.

Show comment
Hide comment
@homu

homu May 8, 2016

⌛️ Testing commit f146230 with merge 21e2229...

homu commented May 8, 2016

⌛️ Testing commit f146230 with merge 21e2229...

@homu

This comment has been minimized.

Show comment
Hide comment
@homu

homu May 8, 2016

☀️ Test successful - status

homu commented May 8, 2016

☀️ Test successful - status

@homu homu merged commit f146230 into new-xkit:master May 8, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@ThePsionic ThePsionic removed the needs review label May 8, 2016

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