Skip to content

Remove the buttons form the image dialoge WIP#102

Merged
thibaudcolas merged 3 commits intowagtail:masterfrom
allcaps:remove-image-save-cancel-btn
Nov 28, 2017
Merged

Remove the buttons form the image dialoge WIP#102
thibaudcolas merged 3 commits intowagtail:masterfrom
allcaps:remove-image-save-cancel-btn

Conversation

@allcaps
Copy link
Copy Markdown
Member

@allcaps allcaps commented Oct 20, 2017

Hello @thibaudcolas,

AltText does reset on close/open. Needs work.

@thibaudcolas thibaudcolas force-pushed the remove-image-save-cancel-btn branch from 17ae710 to 9c6ac17 Compare November 28, 2017 12:22
@thibaudcolas thibaudcolas added the enhancement New feature or request label Nov 28, 2017
@thibaudcolas thibaudcolas added this to the v1.0.0 milestone Nov 28, 2017
@thibaudcolas thibaudcolas self-assigned this Nov 28, 2017
@thibaudcolas thibaudcolas self-requested a review November 28, 2017 12:22
Copy link
Copy Markdown
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

Thank you @allcaps. Sorry it took quite a while for me to get back to this. I wanted to get a new release out with bug fixes before looking into enhancements.

The main reason why we didn't manage to get this working at the sprint is because... there is a bug in Draft.js. We were changing the image entity data but it wasn't reflecting in the UI, because at the moment entity data is a mutable global store and updating that only will fail to trigger a re-render of anything in the <Editor> component, which (wrongly) assumes its input to be immutable.

Long story short, I fixed that by adding a "useless" change on block data.

See facebookarchive/draft-js#839 for further details.


I also changed something somewhat unrelated – in the past, properties altText and alt were used interchangeably to handle the img's alt. This is now using alt everywhere.

@thibaudcolas
Copy link
Copy Markdown
Member

Coverage decrease is because I forgot to ignore a file in master. Will fix that there.

@thibaudcolas thibaudcolas merged commit c2cc937 into wagtail:master Nov 28, 2017
@thibaudcolas thibaudcolas mentioned this pull request Nov 28, 2017
4 tasks
@thibaudcolas thibaudcolas modified the milestones: v1.0.0, Wagtail 2.0 Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants