Skip to content

Comments

dt-multi-text-link component functionality (early version)#192

Open
brady-lamansky-gtt wants to merge 8 commits intoDiscipleTools:masterfrom
brady-lamansky-gtt:multi-text-link
Open

dt-multi-text-link component functionality (early version)#192
brady-lamansky-gtt wants to merge 8 commits intoDiscipleTools:masterfrom
brady-lamansky-gtt:multi-text-link

Conversation

@brady-lamansky-gtt
Copy link
Contributor

@cairocoder01 Here's what @jlamanskygitt & I set up for the new multi-text field type. It extends multi-text since it uses a the same base functionality, but I also re-created some functions from HasOptionsList for the dropdown navigation. I'm sure this isn't the optimal way to handle it. Please let us know what suggestions you have for improvements!

  • tests will need to be written. Currently they're the same as multi-text, except I removed the "add new" test since we got rid of that version of the button

@netlify
Copy link

netlify bot commented Jan 27, 2026

Deploy Preview for jade-chebakia-17493f failed.

Built without sensitive environment variables

Name Link
🔨 Latest commit 3b68354
🔍 Latest deploy log https://app.netlify.com/projects/jade-chebakia-17493f/deploys/699724b4902d830008a08f15

@cairocoder01
Copy link
Collaborator

cairocoder01 commented Jan 28, 2026

  • Since the essense of the component is grouping the multi-text, let's use a more appropriate like dt-multi-text-groups
  • On Empty story, it seems there aren't any groups given to the component because the dropdown does not show any options. If the component has no groups, it shouldn't show the dropdown at all and selecting the add button should just add an item to the default group. Let's add another story to separate these situations. "Empty" should have a set of groups assigned to it. Then we can add another one named "Empty - No Groups" that is like the current Empty story where no groups are specified.
  • Entered value story should show values in multiple groups (should be like the Groups story. We can remove that one and just move it to Entered Value to be consistent with other components)
  • Private icon is not showing next to the label where it should be.
image
  • Loading/Saved icons are not showing at the end of the input
image image
  • Too much space around group titles. Remove padding below group title. Decrease padding above it.
  • Group titles and inputs don't seem to be using the font-family defined in CSS variables and used for all other text that gets displayed. Notice that the label is a sans-serif font but the group titles and placeholders are serif fonts.
image
  • When all the items in a group are removed, the group title is still visible. It shouldn't show if there are no values
image
  • We don't need this field to support password or other non-text types of inputs
  • The dropdown seems to show space on the right side for a scroll bar. I don't think we should need to scroll options. There shouldn't ever be so many groups that we need to scroll. Let's remove the overflow scroll and just show them all.
image
  • After adding one new item, the next time I click the add button, the menu goes away right away. After clicking a few times, I noticed that the menu opens when I click down and closes when I release the mouse button. This doesn't happen the first time you use that button, but it happens after you have added one.
    screencast 2026-01-28 11-14-21

  • Can you add the component to the All Components story so we can test how it interacts with tabbing between components?

Comment on lines 75 to 87
value: {
type: Array,
reflect: true,
},
groups: { type: Array },
open: {
type: Boolean,
state: true,
},
activeIndex: {
type: Number,
state: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't need to redefine properties that are in the parent class if its type hasn't changed. value is in DtMultiText as an array

@cairocoder01
Copy link
Collaborator

cairocoder01 commented Jan 30, 2026

  • The groups dropdown doesn't seem to line up with the plus above it. It's neither centered or right aligned. Can we have the right side of the dropdown line up with the right side of the plus icon?
  • When the groups dropdown is open, if I click outside of it the dropdown should close. Currently it does not.
  • The Empty story doesn't show any inputs. It should show one blank input by default (add test)
  • If there are no groups specified and thus the only group is Default, we shouldn't show the group name. (add test and the inverse)
  • If I remove all items, I'm unable to add any new ones. On Entered Value, remove all items. Then try to add a new one. Nothing happens (add test)
  • On "All Components", if I tab through to the add button and then hit the enter key. Nothing happens. It should add a new item and focus that new input. Works fine when groups are given, but not when they aren't (e.g. "Empty No Groups"). Looks like spacebar works but not enter/return. Let's make sure they both do. (add 2 tests confirming both keys work)
  • The Basic Form story doesn't allow groups besides Default. All stories should have groups specified by default except for the few tests where we want to test it without groups.

@cairocoder01
Copy link
Collaborator

  • The click-anywhere-to-close is mostly working right, but I found a weird edge case where if you click to open the groups dropdown, and then click in the input, the dropdown doesn't close.

Other than that, it's looking good-to-go! Nice work! Let's go ahead and work on the PR for the theme to implement this new component for the Links field type. Make sure to find the old js and remove what isn't needed any more in the process. I'll leave this open until we're sure everything is working in the theme too.

Comment on lines 88 to 89
export const Empty = Template.bind({})
Empty.args = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of the Template.bind legacy syntax, can we update this to the modern JSON object notation?

@cairocoder01
Copy link
Collaborator

@brady-lamansky-gtt This looks like it's ready to go. Let's get the PR for the theme ready to make sure it's working there as well. I'll leave this one open until we can confirm it works as expected in the theme too.

@cairocoder01 cairocoder01 linked an issue Feb 19, 2026 that may be closed by this pull request
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.

Component: Link Field type

2 participants