Sage dynamic select input - select2 #709

kajabijamell · 2021-08-12T16:03:40Z

Description

Adds a select2 input type for sage forms

API Options:

url: String,
label: String,
name: String,
id: String,
has_error: [:optional, TrueClass],
message: [:optional, String],
required: [:optional, TrueClass],
// sets default selected option for the input
default_value: [:optional, String, Integer],
default_text: [:optional, String],
// enables pagination for 25 rows at a time
paginate: [:optional, TrueClass],
// allows user to search through retrieved results
search: [:optional, TrueClass],
// displays an x button for the user to clear the selected value
clear: [:optional, TrueClass],
placeholder: [:optional, String],
// toggles legacy select2 input preset options would be "sage" or "bootstrap"
theme: [:optional, String],

Screenshots

Component States

Empty state

Selected state

Input open w/ search

Input open w/o search

input w/ error

Testing in `sage-lib`

IMPORTANT

This input has dependencies that are only available in Kajabi Products. There's no way to test this from sage-lib.

Testing in `kajabi-products`

Testing is straightforward. You will need a URL generated from rails to deliver the select options via AJAX request. See component sample code here.

Examples of possible URLs can be found in the select_options controller found here in Kajabi Products app/controllers/admin/select_options_controller.rb.

Once you've found a URL to fetch data, you can then play with the different API attributes to reviews the input in different states. Screenshots are provided above as well.

This PR is additive, meaning it will not affect ant current implementation of the select2 inputs. It may be wise to double-check some existing select2 inputs just to make sure there's no overlap in styles or js.

(LOW) Add dynamic select inputs to the sage-lib and does not affect any current components or code structures.

…elop

philschanely

Looking good here, @jamell-kajabi !

Regarding nesting depth, if its necessary (which in vendor situations like this would be understandable!) you can add /* stylelint-disable selector-max-compound-selectors */ before the set of styles that violate the rule and then /* stylelint-enable selector-max-compound-selectors */ afterwards.

One thing that would be great to add is some documentation around this and how it can be used. This is typically adding a folder and the set of files you can see for others inside docs/app/views/examples/components and then an entry for this component in docs/app/helpers/components_helper.rb. Then it will appear in the Components sidebar.

However, I understand following the other examples may not be 100% possible due to the asynchronous nature. It would be great to add the required files, document the props from your sage_dynamic_select.rb file similar to other components. But then if you can't easily show actual uses in the preview file, consider at least providing a code sample like what is shown on the Toast component.

I know this is going to be work well invested since this component is used so much in the app now. Thanks SO MUCh for doing this!! 🙇‍♂️

packages/sage-system/lib/dynamicSelect.js

philschanely · 2021-08-12T20:44:19Z

packages/sage-assets/lib/stylesheets/components/_dynamic_select.scss

+// empty state //
+.sage-dynamic-select {
+  position: relative;
+  &__data {


Could we please move this out onto its own? We tend to avoid this type of use of & in Sage.

.sage-dynamic-select { // ... } .sage-dynamic-select__data { // ... }

For clarity, the goal is to avoid using & for segments of a particular element selector, but OK to use it for a whole element. So:

❌

.block { &__element { // stuff } &--variant { &__element { // stuff } } }

✅

.block__element { // stuff .block--variant & { // stuff } &::after { // stuff } }

And with the BEM naming we'd typically not nest an element selector inside a block selector so that the styles contained in a selector are ideally applied to that actual thing. If specificity or context is needed you bascially flip it so instead of:

.block { &--variant { &__element { // stuff } } }

You do:

.block__element { // stuff .block--variant & { // stuff } }

It is more typing (increased chance for typos 😬 ) but helps ensure each selector block is explicitly applying to that thing, thus preventing style leaks. Everything is also in one place for that particular thing too. Hope this makes sense. 👍 but if not please let me know and we can sync up. 🙌

philschanely · 2021-08-12T20:44:47Z

packages/sage-assets/lib/stylesheets/components/_dynamic_select.scss

+    bottom: sage-spacing(xs);
+    left: 50%;
+  }
+  .select2-container--sage {


Does this need to be nested for sufficient specificity, or could this be pulled out to avoid the extra nesting depth?

philschanely · 2021-08-12T20:47:43Z

packages/sage-assets/lib/stylesheets/components/_dynamic_select.scss

+    height: $-dynamic-select-selected-height;
+    padding: $-dynamic-select-selected-padding;
+    border-color: $-dynamic-select-border-color;
+    .select2-selection__rendered {


Same here... if not needed, can this be pulled out a layer or more to avoid the deep nesting?

kajabijamell · 2021-08-13T15:52:12Z

Thank you for your review @philschanely! I was able to refactor some nesting which definitely helps with readability. I did have some questions regarding adding the input to our docs site, would love to sync with you around this.

teenwolfblitzer

🔥 This is looking great, Jamell! Will be a substantial improvement to our select2 implementation in the app.

In addition to the nesting items that Phil suggested, my comments below are primarily on some cleanup and hopefully clearing up my understanding on derived values.

packages/sage-system/lib/dynamicSelect.js

teenwolfblitzer · 2021-08-13T05:22:20Z

docs/lib/sage_rails/app/views/sage_components/_sage_dynamic_select.html.erb

+      <%= component.generated_html_attributes.html_safe %>
+      name="<%= component.name %>"
+      id="<%= component.id %>"
+      tabindex="-1"


Note for the group (no actionable items as of yet): we'll have to be mindful of this and the aria-hidden="true" below for accessibility. Select2 has been actively working on improving their compliance, but it's not quite there, so we'll have to keep a close watch on it.

packages/sage-assets/lib/stylesheets/components/_dynamic_select.scss

packages/sage-assets/lib/stylesheets/components/_form_select.scss

docs/lib/sage_rails/app/views/sage_components/_sage_dynamic_select.html.erb

teenwolfblitzer

👍🏼 LGTM, a few minor edit tweaks to the preview copy but no major blockers that I can see. After testing with the bridge, I think we may need some additional design input re: the height change for selected values as a follow-up. Great work, this will be a useful addition!

teenwolfblitzer · 2021-08-18T01:19:05Z

docs/app/views/examples/components/dynamic_select/_preview.html.erb

+  clear: "Allow user to clear input value"
+
+  // Example Implementation
+  sage_component SageDynamicSelect, {


As @philschanely had mentioned earlier, ideally we'd have a rendered preview of the component in action, but this works as a substitute.

As a follow-up, I wonder if we could reference the external JS dependencies (jquery and select2) from a CDN just for this preview page. That'd give us a working preview but prevent the dependencies from being packaged up.

We may be able to add them as peer dependencies in the repo. @voodooGQ would this make sense?

teenwolfblitzer · 2021-08-18T01:21:19Z

docs/app/views/examples/components/dynamic_select/_preview.html.erb

+</p>
+
+<h4>Error Handling</h4>
+<p>Much like other sage inputs, the dynamic select includes options to render error messages from the server. By using the <code>has_error</code> attribute, one can trigger the erros states and print an error message that gets placed below the input.</p>


Tiny typo here "error states"

teenwolfblitzer · 2021-08-18T01:21:53Z

docs/app/views/examples/components/dynamic_select/_preview.html.erb

@@ -0,0 +1,42 @@
+<h3>Working with the<code>Sage Dynamic Select Input (Select2)</code></h3>
+<p>
+  The dynamic select input is a sagified version of the select2 input that resides in <code>Kajabi Products.</code> The input fuctionality leverages the Javascript library select2 and includes several options for tailoring the input to the UI. The input can retrieve options from any arbitrary API that will return validly structured JSON.<br/> <br/> Basic usage and complete documentation ca be found


😍 Beautifully written!

One suggestion, we can replace the <br> tags with a new <p>

Note as well that the md() helper may be useful to simplify as well.

teenwolfblitzer · 2021-08-18T01:54:20Z

packages/sage-assets/lib/stylesheets/components/_dynamic_select.scss

+$-dynamic-select-placeholder-color: sage-color(gray, 400);
+$-dynamic-select-color-success: map-get($sage-field-colors, success);
+$-dynamic-select-border-box-shadow-size: map-get($sage-field-configs, box-shadow-size);
+$-dynamic-select-selected-height: rem(54);


We don't have an existing spec in Figma for this, so I won't consider it a blocker but do want to note that the height for the component is much taller (3.375rem) than the standard SageFormSelect field (2.25rem) with an option selected. This shouldn't be an issue for standalone fields, but in cases where inputs are horizontally aligned, the difference will be noticeable:

SageFormSelect SageDynamicSelect

jed1976

This is looking really nice Jamell! Thanks so much for putting the time in to convert this to Sage. Will make an awesome addition to our collection. I left one comment, not a blocker. 👍🏼

packages/sage-system/lib/dynamicSelect.js

…elop

philschanely

Looking in great shape, @jamell-kajabi ! Thanks for your hard work on this!

jamell-kajabi added 4 commits August 12, 2021 08:59

feat(dynamic-select): add markup and ruby class

d2e0895

feat(dynamic-select): add styles for the dynamic select

0ca1a81

feat(form-select): reduce top padding

df17301

feat(dynamic-select): add js and input logic

f9cc302

kajabijamell self-assigned this Aug 12, 2021

jamell-kajabi added 3 commits August 12, 2021 09:05

Merge branch 'develop' of https://github.com/Kajabi/sage-lib into dev…

92f1734

…elop

Merge branch 'develop' into jdj-dynamic-select-input

d86ef39

fix(dynamic-select): fix scss linter errors

efb27eb

kajabijamell requested review from philschanely, teenwolfblitzer and a team August 12, 2021 17:08

kajabijamell changed the title ~~Jdj dynamic select input~~ Sage dynamic select input - select2 Aug 12, 2021

philschanely reviewed Aug 12, 2021

View reviewed changes

jamell-kajabi added 2 commits August 13, 2021 08:44

fix(dynamic-select): fix scss nesting

e3529fd

fix(dynamic-select): update js animation, remove focusout

c4cb6b0

teenwolfblitzer reviewed Aug 13, 2021

View reviewed changes

jamell-kajabi added 4 commits August 13, 2021 13:25

fix(dynamic_select): update spacing values to int

b898bd7

fix(dynamic_select): conditionally render data attrs

707795e

fix(dynamic_select): remove unused function

df2baed

fix(dynamic_select): add documentation and sample code

7b14a8a

kajabijamell requested review from teenwolfblitzer, philschanely and sdhull and removed request for sdhull August 16, 2021 22:04

teenwolfblitzer requested a review from a team August 17, 2021 22:22

teenwolfblitzer approved these changes Aug 18, 2021

View reviewed changes

teenwolfblitzer requested a review from a team August 18, 2021 01:59

jed1976 approved these changes Aug 18, 2021

View reviewed changes

packages/sage-system/lib/dynamicSelect.js Outdated Show resolved Hide resolved

FuturaExtraBold force-pushed the develop branch from c9382c6 to 3731a69 Compare August 18, 2021 16:07

jamell-kajabi added 3 commits August 22, 2021 17:44

Merge branch 'develop' of https://github.com/Kajabi/sage-lib into dev…

015ebc2

…elop

Merge branch 'develop' into jdj-dynamic-select-input

ed386e1

fix(dynamic_select): add param for pagination size

0fbd075

philschanely approved these changes Aug 23, 2021

View reviewed changes

kajabijamell merged commit e38f1ca into develop Aug 23, 2021

kajabijamell mentioned this pull request Aug 23, 2021

Next version bump (~v4.12.0) #738

Merged

5 tasks

teenwolfblitzer added this to the v4.13.x milestone Aug 23, 2021

monicawheeler deleted the jdj-dynamic-select-input branch November 11, 2021 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sage dynamic select input - select2 #709

Sage dynamic select input - select2 #709

kajabijamell commented Aug 12, 2021 •

edited

philschanely left a comment

philschanely Aug 12, 2021

philschanely Aug 12, 2021 •

edited

philschanely Aug 12, 2021

kajabijamell Aug 13, 2021

philschanely Aug 12, 2021

kajabijamell Aug 13, 2021

philschanely Aug 12, 2021

kajabijamell Aug 13, 2021

kajabijamell commented Aug 13, 2021 •

edited

teenwolfblitzer left a comment

teenwolfblitzer Aug 13, 2021

teenwolfblitzer left a comment

teenwolfblitzer Aug 18, 2021

philschanely Aug 23, 2021

teenwolfblitzer Aug 18, 2021

teenwolfblitzer Aug 18, 2021

philschanely Aug 23, 2021

teenwolfblitzer Aug 18, 2021

jed1976 left a comment

philschanely left a comment

Sage dynamic select input - select2 #709

Sage dynamic select input - select2 #709

Conversation

kajabijamell commented Aug 12, 2021 • edited

Description

Screenshots

Component States

Testing in sage-lib

Testing in kajabi-products

philschanely left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philschanely Aug 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kajabijamell commented Aug 13, 2021 • edited

teenwolfblitzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teenwolfblitzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jed1976 left a comment

Choose a reason for hiding this comment

philschanely left a comment

Choose a reason for hiding this comment

kajabijamell commented Aug 12, 2021 •

edited

Testing in `sage-lib`

Testing in `kajabi-products`

philschanely Aug 12, 2021 •

edited

kajabijamell commented Aug 13, 2021 •

edited