Skip to content

Remove the buttons form the image dialoge WIP #102

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

Merged
merged 3 commits into from
Nov 28, 2017

Conversation

allcaps
Copy link
Contributor

@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
Collaborator

@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
Collaborator

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

@thibaudcolas thibaudcolas merged commit c2cc937 into springload: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