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

#351 Inline editable enchancements #366

Merged
merged 2 commits into from Jan 12, 2017
Merged

Conversation

charlieTheBotDev
Copy link
Contributor

@charlieTheBotDev charlieTheBotDev commented Jan 5, 2017

Description

  • Added the enterValueText option to display when no value is entered (see screenshots)
  • Added a value option (defaults to selection.text() as previous)
    • This is so the value can be set when using the fluid version
  • Fixed the change event - it was passing {api: false, value:<value>} regardless of whether the value was set with the api or by the user (now uses standard change/value data)
  • Added focus to the input when the editable is 'opened'
  • Allow pressing enter to set the value
  • Added tests for full coverage

Motivation and Context

The initial scope of this was to resolve #351 but in the process of resolving it and adding tests I noticed a few other bugs

How Has This Been Tested?

Added tests for full coverage of the module, tested on the demo page example

Screenshots (if appropriate):

screen shot 2017-01-05 at 09 12 31
screen shot 2017-01-05 at 09 12 35
screen shot 2017-01-05 at 09 11 58
screen shot 2017-01-05 at 09 12 03

Checklist:

  • I have added a tag to this pull request that indicates the impact of the change (patch, minor or major)
  • My code follows the code style of this project.
  • I have updated the documentation accordingly. (This includes updating the changelog).
  • I have read the CONTRIBUTING document.
  • All my changes are covered by tests.
  • This request is ready to merge

@coveralls
Copy link

coveralls commented Jan 5, 2017

Coverage Status

Coverage increased (+0.3%) to 61.211% when pulling cc6dc90 on #351-inline-editable-enhancements into 6ccc864 on master.

Copy link
Contributor Author

@charlieTheBotDev charlieTheBotDev left a comment

Choose a reason for hiding this comment

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

I think the changes to the change event data and the addition of the value/enterValueText options warrant this being a 'minor' instead of a 'patch'.
It does fix several bugs but it's mainly enhancements

@@ -63,7 +63,7 @@ class InlineMorphSection extends MorphSection
morphSection.detector = undefined
morphSection.hide()
@on 'hide', 'hx.morph-section', (data) ->
exitEditMode.call(morphSection, data.toggle, data.content)
exitEditMode?.call(morphSection, data.toggle, data.content)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you don't have to pass it through as an empty function if you don't have an exit edit mode function

}

.hx-inline-editable-no-value {
font-style: italic;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making it italic when no value has been entered is probably enough of a visual cue (along with the Enter Value text) that the user should do something


value: (value) ->
if value isnt undefined
@textSelection.text(value)
@emit('change', {api: false, value: value})
@emit('change', { cause: 'api', value: value })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was broken anyway (api was always passed as false) so I updated to the cause/value mentality.

@jmsmyth jmsmyth merged commit 26b4147 into master Jan 12, 2017
@jmsmyth jmsmyth removed the In Review label Jan 12, 2017
@charlieTheBotDev charlieTheBotDev deleted the #351-inline-editable-enhancements branch February 7, 2017 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InlineEditable: no way to edit empty string
3 participants