Skip to content

Comments

Studio-frontend in-context image selection modal#17834

Merged
efischer19 merged 1 commit intomasterfrom
thallada/incontext-waffle
Apr 5, 2018
Merged

Studio-frontend in-context image selection modal#17834
efischer19 merged 1 commit intomasterfrom
thallada/incontext-waffle

Conversation

@thallada
Copy link
Contributor

@thallada thallada commented Mar 29, 2018

Adds a new waffle switch: studio.enable_in_context_image_selection. If on, Studio will load the studio-frontend code for the editImageModal component and inserts it into the DOM. The TinyMCE insert image toolbar button will send a signal to open up the modal when the waffle switch is on instead of opening the old modal.

The studio-frontend PR should be merged and a new release should be made before this PR is merged.

Sandbox with this branch: https://studio-thallada.sandbox.edx.org/ Try editing an HTML component in a unit.

@edx/educator-dahlia

@thallada thallada force-pushed the thallada/incontext-waffle branch from e363546 to f859d3b Compare March 29, 2018 19:52
@edx-status-bot
Copy link

Your PR has finished running tests.

@thallada thallada force-pushed the thallada/incontext-waffle branch from f859d3b to d14ec65 Compare March 30, 2018 15:06
@edx-status-bot
Copy link

Your PR has finished running tests.

Copy link
Contributor

@dsjen dsjen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I left a few questions and also there's the baseURL and static link issues we ran into earlier today. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be dynamic instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be wrapped in gettext for translations?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: might be a little cleaner by setting it all at once?

imgAttrs = {
  src: img.attr('src'),
  alt: img.attr('alt'),
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the baseAssetUrl stuff, I'm now initializing imgAttr like this:

imgAttr =
  baseAssetUrl: @base_asset_url

So, I can't just initialize it in that if condition now. Unfortunately, the coffeescript compiler the platform is using doesn't seem to support the object spread syntax which would allow me to clean that code up. E.g.

imgAttr =
  imgAttr...
  src: img.attr('src')
  ... etc ...

I get a syntax error if I try that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok!

package.json Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will need to be updated when a new version of SFE is released!

@thallada thallada force-pushed the thallada/incontext-waffle branch 4 times, most recently from 0adc889 to cd1aa68 Compare March 30, 2018 21:52
@edx-status-bot
Copy link

Your PR has finished running tests.

@thallada thallada force-pushed the thallada/incontext-waffle branch from cd1aa68 to 90f3602 Compare April 2, 2018 15:43
@edx-status-bot
Copy link

Your PR has finished running tests.

2 similar comments
@edx-status-bot
Copy link

Your PR has finished running tests.

@edx-status-bot
Copy link

Your PR has finished running tests.

@thallada thallada force-pushed the thallada/incontext-waffle branch 2 times, most recently from 172bd5f to 45b4e0d Compare April 4, 2018 18:31
@edx-status-bot
Copy link

Your PR has finished running tests.

Open edit image modal via a custom signal

Send img tag attributes in open signal

Send natural image dimensions if no attr specified

Add new modal button in addition to old button

Also load built CSS when waffle switch on

Swap out TinyMCE toolbar button on waffle switch

Pass LANGUAGE_CODE from Django to template to studio-frontend

Define request.LANGUAGE_CODE in test_container_page.py
@thallada thallada force-pushed the thallada/incontext-waffle branch from 45b4e0d to d31249b Compare April 4, 2018 19:48
@edx-status-bot
Copy link

Your PR has finished running tests.

@fysheets
Copy link

fysheets commented Apr 5, 2018

jenkins run lettuce

@fysheets
Copy link

fysheets commented Apr 5, 2018

jenkins run quality

@edx-status-bot
Copy link

Your PR has finished running tests.

@dsjen
Copy link
Contributor

dsjen commented Apr 5, 2018

HOORAY!!!!!!!

@efischer19 efischer19 merged commit 32f9902 into master Apr 5, 2018
@efischer19 efischer19 deleted the thallada/incontext-waffle branch April 5, 2018 14:34
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Friday, April 06, 2018.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been rolled back from the production environment.

feanil added a commit that referenced this pull request Oct 16, 2025
This is only needed to make the legacy editor work on containers. Added
originally via #17834

However, now that we use the new MFE text editor even on this legacy
page, we don't need to load the old image modal in the old text editor.
feanil added a commit that referenced this pull request Oct 16, 2025
This is only needed to make the legacy editor work on containers. Added
originally via #17834

However, now that we use the new MFE text editor even on this legacy
page, we don't need to load the old image modal in the old text editor.
naincy128 pushed a commit to edx/edx-platform that referenced this pull request Oct 27, 2025
This is only needed to make the legacy editor work on containers. Added
originally via openedx#17834

However, now that we use the new MFE text editor even on this legacy
page, we don't need to load the old image modal in the old text editor.
naincy128 pushed a commit to edx/edx-platform that referenced this pull request Oct 27, 2025
This is only needed to make the legacy editor work on containers. Added
originally via openedx#17834

However, now that we use the new MFE text editor even on this legacy
page, we don't need to load the old image modal in the old text editor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants