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

WIP -- inline widgets & block editing modals #301

Merged
merged 167 commits into from Jan 25, 2017
Merged

WIP -- inline widgets & block editing modals #301

merged 167 commits into from Jan 25, 2017

Conversation

forresto
Copy link
Contributor

@forresto forresto commented Dec 20, 2016

pr goals... net negative loc

  • disable tooltips & tag minor
  • update libs (stable api)
  • react widgets inline
  • iframe widgets inline
  • widgets NodeView perf
  • fix ed store mutations
  • redo h1/p.empty placeholder plugin
  • redo shareurl plugin
  • redo fixed menu hack plugin
  • redo command interface plugin
  • tests
  • release rc
  • pm 0.17.0
  • fix file drops
  • release major (no api change for clients) ------------

@narrowdesign QA finds:

  • getting 404 on code and map
  • Can’t add attributions but can delete the ones you slugged in (Cole Rise etc). Can’t toggle ✅
  • no feedback when clicking > add X from images or quotes.
  • It either feels weird to be able to add a link to a word in an image title/quote or weird that it doesn’t work after I do it.
  • clicking when focus is inside an image or another quote creates a quote wherever the cursor was before last focus change.

I see some jumping in Safari, but it's Safari bugs that I'm working around with the positioning hacks. ~~~Going to roll with it for now.~~~

  • fix so that only iOS uses the hacks and other browsers are smoother

1/20 @narrowdesign QA:

  • IMAGE looks like it should link to something. Not sure its purpose here.
    image
  • Pressing return while in an image attribution text input should close the input menu
  • Turn off text selection on draggable image blocks
    image
  • In the hint, use explicit Link must start with http:// or https:// for normals or you'll get a lot of people trying to type http...
  • "Are you sure" doesn't feel like an action. Needs to be in your face and clear that it's final. "Delete forever"
  • This is easy to miss and just click away from after hitting delete
    image
  • Clicking "Are you sure" should close the modal since that item no longer exists at that point.
  • when clicking link icon, the inputs appear in a fixed spot near the top of the app even when scrolled way down
  • Tall image attribution links are hidden behind the footer bar and can't scroll more
    image
  • Quote blocks don't look like quotes or have a label telling me they are quotes unless in edit mode.
  • Clicked quote several times before I realized it was creating new boxes w/ an edit button. Should open that edit menu and set focus to the input
  • Presence of attributions is undetectable in collapsed view
  • Image shows text but modal shows nothing but attributions (the words in the collapsed view aren't in the attributions either)
    image
    image

Not regressions / not blockers

  • Can't clear paragraph break when photo is the first thing there is
    image
  • Can't click between images to add text. (I see that you're adding paragraph breaks between them. But I delete those instinctively every time. Now that I know why they are there I won't.)
  • Rolling over linked text shows pointer but doesn't give an option to link
  • Links in text aren't editable. Clicking the link button turns the link off and has to be redone.

Webapp issues

  • Can't type a full quote attribution link without it clearing what I am typing before it is a valid link. Was able to type http://cnn.com once and it stuck. Must be clearing if not valid after Nseconds.
  • Image load bar doesn't clear after loaded. Also there in the edit mode.
    image
  • Hide the close button when modal is open, it's easily confused for "close modal" but it takes you all the way out to cards view.
  • Link menu looks inactive (tried a couple to be sure it wasn't the color scheme's issue
    image
    image

this is too much for one commit, sorry. everything broke, fixed basic
text editing and formatting. to fix:
* widgets
* menu
# Conflicts:
#	src/components/editable-menu.css
#	src/components/editable.js
#	src/menu/menu-media.js
# Conflicts:
#	package.json
#	src/components/app.js
#	src/components/dropdown-group.js
This was referenced Dec 20, 2016
"rules": {
"no-multiple-empty-lines": [2, {"max": 2, "maxEOF": 1}],
"comma-dangle": [2, "always-multiline"],

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should not have done it in the same PR tho 😳

"babel-eslint": "6.1.2",
"babel-loader": "6.2.4",
"babel-preset-es2015": "6.9.0",
"babel-cli": "6.18.0",

Choose a reason for hiding this comment

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

👍

@@ -26,7 +27,7 @@ export default function (items) {
if (el) container.appendChild(el)
})
}
return EdSchema.parseDOM(container)
return DOMParser.fromSchema(EdSchema).parse(container)

Choose a reason for hiding this comment

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

Nice naming cleanup


const rules = inputRules.allInputRules.concat(edRules)
// const isIOS = /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream

Choose a reason for hiding this comment

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

no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With pm15... https://discuss.prosemirror.net/t/release-0-15-0/519

The main change in this release is that a bunch more things (typing, in-textblock backspacing) are left to the browser, and ProseMirror takes care not to re-render content when it doesn't have to. Which goes exactly against the original concept of ProseMirror ('never trust the browser, we'll do it ourselves'), but more and more issues came up where browser or platform features, such as spell-check, long-press to type character variants on OS, double-space for a period on iOS, were broken because the browser and/or the OS didn't realize what was going on.

So for relatively uncontroversial functionality (there's not too much disagreement on what should happen to your document when you type a character), the editor now takes a hands-off approach.

@forresto
Copy link
Contributor Author

@narrowdesign

ed-p-insert

  • you can select an empty ¶ with focus + escape then backspace to delete
  • you can add a paragraph between images selecting one then pressing enter
    • unless image is first block, then it will add empty ¶ before it

These are ProseMirror default behaviors. Would there be a better path?

@forresto
Copy link
Contributor Author

(Though that doesn't help mobile, where OSK disappears on block selection. Menu items for "delete block" & "add paragraph above" would help.)

@forresto
Copy link
Contributor Author

Almost down to polish.

screen shot 2017-01-24 at 5 04 09 pm

@forresto forresto merged commit 89cf23a into master Jan 25, 2017
@forresto forresto deleted the pm-0-11-1 branch January 25, 2017 21:43
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.

None yet

3 participants