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

Bug fixes and updates to latest conventions for sortable component #195

Merged
merged 1 commit into from Oct 24, 2017

Conversation

jcredding
Copy link
Member

@jcredding jcredding commented Oct 23, 2017

This updates the sortable component to fix some bugs introduced
while updating it to no longer use jquery. This also includes
updates for the sortable component to make it match some of our
latest conventions. This is part of updating Romo to no longer
require jquery.

This fixes the Romo.before call when the dragged elem is
dropped. It was accidentally updated to insert the placeholder
elem before the dragged elem instead of inserting the dragged
elem before the placeholder elem. This was incorrectly updated
in PR 161 when jquery was removed. It previously used
insertBefore on the dragged elem and is passed the placeholder
elem. This is the opposite order for how the elems needed to be
passed to Romo.before which is what caused the issue.

This updates the dragging, drag over, and placeholder class to
be lazy methods and also removes defaulting their values. This
is because the addClass and removeClass helpers error when
using an empty string which was the previous default. Now the
sortable component checks if it has the class before trying to
add or remove it which is consistent with these being optional.
This makes the methods lazy to match our latest conventions to
avoid all data attrs having to be set when the component is
initialized.

This updates the sortable to use undefined instead of null
for its attributes that are only set when dragging an element. The
other romo components have centered around using undefined so
the sortable component should as well. This also updates a few of
the if to check for undefined instead of using falsey values.
This is more explicit and should help avoid logic seemingly
working with invalid values.

This also fixes a few typos on method names. These were
accidentally introduced when the methods were renamed to our
private nameing scheme.

@kellyredding - Ready for review. I believe this gets the sortable component fully working.

@kellyredding
Copy link
Member

@jcredding "shoudl" in commit msg.

Copy link
Member

@kellyredding kellyredding left a comment

Choose a reason for hiding this comment

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

@jcredding looks good - I like the more explicit nature of this and what we are moving to in general 👎

This updates the sortable component to fix some bugs introduced
while updating it to no longer use jquery. This also includes
updates for the sortable component to make it match some of our
latest conventions. This is part of updating Romo to no longer
require jquery.

This fixes the `Romo.before` call when the dragged elem is
dropped. It was accidentally updated to insert the placeholder
elem before the dragged elem instead of inserting the dragged
elem before the placeholder elem. This was incorrectly updated
in PR 161 when jquery was removed. It previously used
`insertBefore` on the dragged elem and is passed the placeholder
elem. This is the opposite order for how the elems needed to be
passed to `Romo.before` which is what caused the issue.

This updates the dragging, drag over, and placeholder class to
be lazy methods and also removes defaulting their values. This
is because the `addClass` and `removeClass` helpers error when
using an empty string which was the previous default. Now the
sortable component checks if it has the class before trying to
add or remove it which is consistent with these being optional.
This makes the methods lazy to match our latest conventions to
avoid all data attrs having to be set when the component is
initialized.

This updates the sortable to use `undefined` instead of `null`
for its attributes that are only set when dragging an element. The
other romo components have centered around using `undefined` so
the sortable component should as well. This also updates a few of
the `if` to check for `undefined` instead of using falsey values.
This is more explicit and should help avoid logic seemingly
working with invalid values.

This also fixes a few typos on method names. These were
accidentally introduced when the methods were renamed to our
private nameing scheme.
@jcredding jcredding merged commit 188a65a into feature-romo-js Oct 24, 2017
@jcredding jcredding deleted the jcr-sortable-fixes branch October 24, 2017 15:50
kellyredding added a commit that referenced this pull request Nov 17, 2017
* add base js `Romo.is` method #139
* add a base js `parents` method #141
* Add `Romo.assign` util, use to merge javascript objects #140
* update how get/set attr and get/set data work for consistency #143
* Hotfix: Return parsed JSON value with `Romo._deserializeValue` 49685d7
* Hotfix: Update `romo-btn` to have no border radius by default 0593d25
* rework the parent-child elems handler to use Romo vanilla js #142
* redesign/rework the init UI elems system #144
* update base prev/next methods to take an optional selector #145
* (hotfix) cleanup minor base js stuff for vanilla js 7de627f
* update base param method to use vanilla js #146
* update base js parse z-index logic to use vanilla js #147
* (hotfix) turn on Romo custom events for event handling 352a584
* Fix binding and unbinding to events on the window/document #152
* rework romo form js to not use jquery #156
* Update `RomoAjax` component to not use jquery #148
* Update `RomoDropdown` component to not use jquery #151
* Update `RomoModal` component to not use jquery #153
* Update `RomoTooltip` component to not use jquery #154
* Update `RomoInline` component to not use jquery #157
* Update `RomoOnkey` component to not use jquery #158
* (hotfix) remove base js TODO comment about reworking for jQuery ad01def
* onkey: add a delay feature; cleanup form and opt list dropdown usage #159
* romoAjax: some cleanups to get up to modern conventions #160
* romoDropdown: cleanups to get up to modern conventions #163
* romoModal: cleanups to get up to modern conventions #165
* Update `RomoWordBoundaryFilter` component to not use jquery #164
* romoTooltip: cleanups to get up to modern conventions #167
* romoInline: cleanups to get up to modern conventions #168
* Update `RomoOptionListDropdown` component to not use jquery #162
* Update `RomoIndicatorTextInput` component to not use jquery #155
* Update `RomoCurrencyTextInput` component to not use jquery #149
* Update `RomoSortable` component to not use jquery #161
* Update `RomoDatepicker` component to not use jquery #150
* (hotfix) some minor style/convention cleanups to OptionListDropdown 780dc6c
* (hotfix) romoIndicatorTextInputs: cleanups for modern conventions c683b1a
* romoSortable: cleanups to get up to modern conventions #170
* romoDatepicker: cleanups to get up to modern conventions #171
* romoCurrencyTextInput: cleanups to get on modern conventions #172
* Update `RomoSelectedOptionsList` component to not use jquery #166
* Update `RomoSelectDropdown` component to not use jquery #169
* (hotfix) remove jquery trigger on RomoOnkey that we missed fc40526
* rework the indicator component as a RomoSpinner component #175
* add "rm" methods for removing attr/data/style #176
* Update `Romo.array` helper to cast non array-like values to arrays #177
* Update `RomoSelect` component to not use jquery #173
* Update `RomoPicker` component to not use jquery #174
* base js: make event handling error if trying to use with elem collections #180
* Update `RomoDropdownForm` component to not use jquery #178
* Update `RomoModalForm` component to not use jquery #179
* Update `RomoInlineForm` component to not use jquery #181
* base js: remove `getComputedStyle` method #183
* update select/picker related components to get up to convention #182
* update modal/dropdown/inline form components to latest conventions #184
* form: convention updates and a couple of bugfixes #185
* Cleanups from the switch to no longer use jquery #186
* add `.updateText` base js method; switch to using it in components #187
* Hotfix: Fix using `RomoDropdown` in inline, modal, and tooltip js 3ecd151
* Hotfix: Fix typo in `parseZIndex` 96f75a0
* Don't use selectors that start with `>` #188
* Update dropdown and modal to not always expect a `contentElem` #189
* Update append/prepend, before/after, and update to allow single elem #190
* Hotfix: Fix typos in select and dropdown logic 0fe8b9b
* Hotfix: Pass spinner elem when adding/removing styles ddc89fa
* Fix select displaying an empty text option as selected #191
* Fix dropdowns placing their popup elem #192
* Update `ParentChildElems.remove` to ignore non element nodes #193
* make modal/dropdown bind needed by romo-av public #194
* Bug fixes and updates to latest conventions for sortable component #195
* Update `Romo.children` and `Romo.siblings` to take optional selector #196
* Update the add/change HTML methods to error when passed non HTML #197
* Fix ` ` usage for select and picker family of components #199
* Don't set `undefined` data attrs #200
* Update option list dropdown to properly display options #198
* Fix using window height/width, use document elem client height/width #201
* Hotfix: Fix closing dropdowns ae43d45
* Hotfix: Pass submit elems when initializing a form 148271c
* Fixes and cleanups related to selects #202
* Fixes and cleanups related to modals #203
* Fixes and cleanups for tooltips #204
* Fixes and cleanups related to pickers #205
* Update `Romo.offset`, don't use `document.body` #206
* Fix event binding for the same elem, event type, and function #207
* Event propagation cleanups #208
* Hotfix: Fix `setData` calls that use `data-` in attr name 3d7b1f1
* (hotfix) jux onkey delay ms config with other onkey configs 49ca1f3
* Update dropdowns, modals, and tooltips to use `setTimeout` to place popup #209
* Fix parsing z-index from parent elems #210
* Fix `Romo.elems` working for tr, th, td, and other table related tags #211
* Fixes and cleanups related to inlines #212
* Fixes and cleanups related to datepickers #213
* update `{add|remove|toggle}Class` methods to take space-separated list #214
* (hotfix) modal: properly add "drag" class to the modal drag elem 531ece4
* (hotfix) update base `parents` meth to work with the body elem 693b1a5
* (hotfix) don't call init ui callbacks in timeouts 1dc4e1a
* selects: update to handle zero options #216
* update how dropdown and tooltip popups are placed in modals #215
* base: update `array` to work with select elems #217
* Hotfix: Fix passing strings to `Romo.array` 69d0b7a
* Fix spinner resetting HTML when stopped #219
* Simplify `setTimeout` calls, don't create extra anonymous functions #220
* Update `Romo.ajax` and `Form` to handle additional scenarios #221
* (hotfix) base: jux events related logic after DOM manipulation logic 7d2081f
* (hotfix) base: rename remove node vars in parent-child elems code 9bb437a
* Update `Romo.array`, short circuit if passed a `Node` #223
* base: update `find` to optionally take a list of parent elems #222
* base: add init elems methods #224
* base: rework elems init API #227
* Update attr, style, and class helpers to allow multiple elems #225
* Update `Romo.remove` to take multiple elems #226
* Hotfix: Fix typo for methods that now take multi elems 3166a01
* Update `on`, `off, and `trigger` to take multiple elems #228
* Update `Datepicker` to pass multi elems to `Romo.on` #229
* Update `Form` to pass multi elems to `on` and `trigger` #230
* Update dropdown, modal, and inline to pass multi elems to helpers #231
* Hotfix: Update `Sortable` to pass multi elems to `removeClass` 7e56fe4
* Update `IndicatorTextInput` to pass multi elems to romo helpers #232
* Update select/picker related components to pass multi elems to romo helpers #233
* Update init replace methods to return single elem #234
* picker/select/select-dropdown: properly bind label click events #235
* (hotfix) ajax: fix `.rmAttr` method name typo/mispelling d572036
* (hotfix) picker: update copy/paste error from select js 52c3603
* base js: fix Romo.elems to be more robust, work with TR strings #237
* currency text input: add onkey to update hidden input on key #236
* (hotfix) dropdown/modal/tooltip: place popup immediately and in timeout 3bfe7c6
* js: reorg event handler methods into event functions sections #238
* add API for defining RomoComponents #239
* (hotfix) update all components to use new RomoComponent api 17f7100
* (hotfix) properly return event function return values 5f29422
* add a RomoPopupStack helper for managing modal/dropdown/etc popups #240
* fix dropdown/tooltip placement in scrollable UI #241
* Hotfix: Handle removing elems with no parent a9f2083
* (hotfix) some cleanups after the popup stack rework 94db42f
* option list dropdown: don't open on focus on clicks #243
* Update dropdowns/modals/tooltips to set max-height then position #242
* restore option list dropdown focusing when closed by esc key #244
* Update romo ajax, don't send empty form data #245
* add `Romo.pushFn` method to formalize set timeout reactor push #246
* make add child elems to parents always run in a pushFn call #247
* (hotfix) popup stack: handle the case where esc closes no popup 612b46c
* Hotfix: Add numbers to `Romo.elems` regex for finding tag names 355babe
* (hotfix) tooltip: allow adding content with incomplete html 6936060
* Hotfix: Update inline's to not always expect a toggle elem 44abcfa
* pickers/selects: update to properly calc caret width/padding #248
* (hotfix) cleanup some Datepicker trigger event namespaces 6dab161
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

2 participants