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

Adds dynamic labels to form with data-label-* #3598

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

HazelGrant
Copy link
Contributor

@HazelGrant HazelGrant commented Jun 7, 2024

Fixes #3327

form.yml

form:
  - node_type
  - cores

attributes:
  node_type:
    widget: select
    options:
      - [ 'small',  'small',  data-label-cores: 'Number of Cores (1-4)'  ]
      - [ 'medium', 'medium', data-label-cores: 'Number of Cores (1-8)'  ]
      - [ 'large',  'large',  data-label-cores: 'Number of Cores (1-16)' ]

  cores:
    widget: "number_field"
    required: true
    value: 1
Screen.Recording.2024-06-07.at.3.57.47.PM.mov

@HazelGrant HazelGrant marked this pull request as ready for review June 10, 2024 12:38
Comment on lines +207 to +211
function newLabel(changeElement, key) {
const selectedOptionLabelIndex = changeElement[0].selectedIndex;
const selectedOptionLabel = changeElement[0].options[selectedOptionLabelIndex];
return selectedOptionLabel.dataset[key];
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this only work for select options - I feel like we should support others like checkboxes. That's the only other type off the top I think this should support.

We can defer it, but it seems to me that someone may want checkbox support - and maybe other input types that I can't think of right now.

@johrstrom johrstrom self-requested a review June 11, 2024 19:48
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

This works as expected. I'm happy to merge now as is and support things like checkboxes later or we can implement now. I'll leave it to you to decide.

@HazelGrant HazelGrant merged commit e1a9549 into master Jun 13, 2024
23 checks passed
@HazelGrant HazelGrant deleted the 3327-dynamic-labels-for-bc branch June 13, 2024 12:49
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.

batch connect support for dynamic labels
3 participants