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

IFrame Option for Lightbox Links #1573

Closed
wants to merge 4 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@ChrisHougard
Contributor

ChrisHougard commented Nov 26, 2014

For #1564. The existing code is pretty buggy. Adding links works but editing does not work at all. I'll look into fixing that later today.

Should I have this auto select the iframe option if a JSONP request would be needed?

@aembler

This comment has been minimized.

Show comment
Hide comment
@aembler

aembler Nov 26, 2014

Member

Sure, that would probably make the most sense.

On Tue, Nov 25, 2014 at 11:05 PM, Christopher Hougard <
notifications@github.com> wrote:

For #1564 #1564. The
existing code is pretty buggy. Adding links works but editing does not work
at all. I'll look into fixing that later today.

Should I have this auto select the iframe option if a JSONP request would

be needed?

You can merge this Pull Request by running

git pull https://github.com/ExchangeCore/concrete5-5.7.0 1564

Or view, comment on, or merge it at:

#1573
Commit Summary

  • Only set midClick to true for ajax. IFrames and images should be
    fine alone.
  • IFrame lightbox option
  • Removed lightbox-image which should help fix bugs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#1573.

Member

aembler commented Nov 26, 2014

Sure, that would probably make the most sense.

On Tue, Nov 25, 2014 at 11:05 PM, Christopher Hougard <
notifications@github.com> wrote:

For #1564 #1564. The
existing code is pretty buggy. Adding links works but editing does not work
at all. I'll look into fixing that later today.

Should I have this auto select the iframe option if a JSONP request would

be needed?

You can merge this Pull Request by running

git pull https://github.com/ExchangeCore/concrete5-5.7.0 1564

Or view, comment on, or merge it at:

#1573
Commit Summary

  • Only set midClick to true for ajax. IFrames and images should be
    fine alone.
  • IFrame lightbox option
  • Removed lightbox-image which should help fix bugs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#1573.

@KorvinSzanto

This comment has been minimized.

Show comment
Hide comment
@KorvinSzanto

KorvinSzanto Dec 5, 2014

Member

This looks good to me. @EC-Chris what do you mean by a JSONP request? How would you detect JSONP?

Member

KorvinSzanto commented Dec 5, 2014

This looks good to me. @EC-Chris what do you mean by a JSONP request? How would you detect JSONP?

@ChrisHougard

This comment has been minimized.

Show comment
Hide comment
@ChrisHougard

ChrisHougard Dec 5, 2014

Contributor

@KorvinSzanto I was planning on checking if the domain is the same as the current domain.

If someone could take a stab at editing that would be helpful. I'm having a hard time figuring out why it doesn't work.

Contributor

ChrisHougard commented Dec 5, 2014

@KorvinSzanto I was planning on checking if the domain is the same as the current domain.

If someone could take a stab at editing that would be helpful. I'm having a hard time figuring out why it doesn't work.

@aembler

This comment has been minimized.

Show comment
Hide comment
@aembler

aembler Dec 23, 2014

Member

This ready to be looked at or is still in limbo?

Member

aembler commented Dec 23, 2014

This ready to be looked at or is still in limbo?

@ChrisHougard

This comment has been minimized.

Show comment
Hide comment
@ChrisHougard

ChrisHougard Dec 23, 2014

Contributor

Link editing still doesn't work. I haven't had time to look in to it.

Contributor

ChrisHougard commented Dec 23, 2014

Link editing still doesn't work. I haven't had time to look in to it.

@ChrisHougard

This comment has been minimized.

Show comment
Hide comment
@ChrisHougard

ChrisHougard Jan 23, 2015

Contributor

Going to try to actually finish this over the weekend.

Contributor

ChrisHougard commented Jan 23, 2015

Going to try to actually finish this over the weekend.

@aembler

This comment has been minimized.

Show comment
Hide comment
@aembler

aembler Jan 24, 2015

Member

Cool!

On Fri, Jan 23, 2015 at 10:54 AM, Christopher Hougard <
notifications@github.com> wrote:

Going to try to actually finish this over the weekend.


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

Member

aembler commented Jan 24, 2015

Cool!

On Fri, Jan 23, 2015 at 10:54 AM, Christopher Hougard <
notifications@github.com> wrote:

Going to try to actually finish this over the weekend.


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

@aembler

This comment has been minimized.

Show comment
Hide comment
@aembler

aembler Mar 24, 2015

Member

This is resolved in feature/redactor-10.

Member

aembler commented Mar 24, 2015

This is resolved in feature/redactor-10.

@aembler aembler closed this Mar 24, 2015

@joe-meyer joe-meyer deleted the ExchangeCore:1564 branch Apr 27, 2015

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