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

Fix bug in which we could not insert images or links using the editor #1

Merged
merged 3 commits into from
Dec 11, 2018

Conversation

JavierGelatti
Copy link

@JavierGelatti JavierGelatti commented Dec 7, 2018

Problem

Editing description to insert external link and images is not working on Teespring.

Steps to reproduce

  1. Go to the listing page in your seller dashboard.
  2. Find any listing (created in Composer or Launcher) and click on the "pencil" icon for "edit". (ie: https://teespring.com/campaigns/72696073/edit)
  3. In this view, try to add in a hyperlink or an image by clicking on the buttons.

Expected behavior

  • You are able to add images and hyperlinks through our description tool editor

Actual behavior

  • The buttons do not respond, and you are not able to add images or hyperlinks through listing edits or Admin campaign pages.

Found issue

When we click on buttons "insert link" or "insert image", the following JS error was raised:
Uncaught TypeError: Failed to execute 'focus' on 'HTMLElement': parameter 1 ('options') is not an object..

This was a known issue, that was fixed in newer versions of the bootstrap-wysihtml5 gem (see: jhollingworth/bootstrap-wysihtml5#391). Currently, we are using a fork from the 0.3.1.24 version, and the latest version of the original gem is 0.3.3.8

Solution

As we on Teespring are using a fork of that gem, upgrading the gem is not direct. Also, I tried to upgrade the gem locally (using the original gem instead of the fork), but there were a bunch of errors during upgrade that we'd need to solve.

So, I decided to solve the problem manually on our fork of the gem.
The problem was solved by replacing element.focus(false) calls with element.focus({}), because the focus function expects an object as its argument (see: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus and jhollingworth/bootstrap-wysihtml5#391 (comment)).

Additional changes

While testing this, @helenlam requested the following:

  • Change the text 'Insert image' to 'Insert image URL'.
  • Make 'http://' a placeholder, instead of the actual initial value of the fields.

Those changes are also included in this PR, in separate commits.

Link to ticket

https://www.pivotaltracker.com/story/show/162432786

Copy link

@niklasnordlund niklasnordlund left a comment

Choose a reason for hiding this comment

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

wysi, wysi, wysi, wizard!

@JavierGelatti JavierGelatti merged commit bb68dfc into master Dec 11, 2018
@JavierGelatti JavierGelatti deleted the platform#162432786-fix_insert_images branch December 11, 2018 15:22
@todditron
Copy link

Did anyone find this patch breaks the ability to edit existing links? Not that the edit link functionality worked all that well before, but at least it would clear the link and let you place a new one ...

I get Uncaught TypeError: Cannot read property 'text' of null

image

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.

3 participants