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

Fixes issue with having to click the delete icon for attachment twice. #892

Merged
merged 1 commit into from Jun 17, 2018

Conversation

2 participants
@Abijeet
Member

Abijeet commented Jun 17, 2018

Fixes #884

This is happening because -

Due to the limitations of modern JavaScript (and the abandonment of Object.observe), Vue cannot detect property addition or deletion. Since Vue performs the getter/setter conversion process during instance initialization, a property must be present in the data object in order for Vue to convert it and make it reactive.

Source: https://vuejs.org/v2/guide/reactivity.html

Also added padding to the icons in the attachment section.

Before
attachment-edit-no-padding

After
attachment-edit-padding

Signed-off-by: Abijeet abijeetpatro@gmail.com

Fixes issue with having to click the delete icon for attachment twice.
Fixes #884

This is happening because -

Due to the limitations of modern JavaScript (and the abandonment of Object.observe), Vue cannot detect property addition or deletion. Since Vue performs the getter/setter conversion process during instance initialization, a property must be present in the data object in order for Vue to convert it and make it reactive.

Source: https://vuejs.org/v2/guide/reactivity.html

Also added padding to the icons in the attachment section.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

@Abijeet Abijeet requested a review from ssddanbrown Jun 17, 2018

@Abijeet Abijeet self-assigned this Jun 17, 2018

@Abijeet Abijeet added this to the BookStack Beta v0.23.0 milestone Jun 17, 2018

@Abijeet Abijeet added the Bug label Jun 17, 2018

@ssddanbrown

@Abijeet Ah, Yeah, I've made this same mistake a few times before. Tested, All works. Thanks for the design fixes as well. Feel free to merge. 👍

@Abijeet Abijeet merged commit 448068e into master Jun 17, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ssddanbrown ssddanbrown deleted the fix/884 branch Dec 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment