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

Add field metadata to MembershipType schema info (xml) #12124

Merged
merged 1 commit into from
May 14, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Per our general code approach - add information as to how to render fields to the MembershipType.xml file for that entity

Before

No change

After

No change

Technical Details

@seamuslee001 @colemanw @tschuettler @mattwire @monishdeb

But, if I start to add these fields to the form using 'addField' instead we get a change of the field labels from

screenshot 2018-05-14 13 01 37

To

screenshot 2018-05-14 13 00 19

The issue is the 'title' fields are defined with quite a bit of context as they might be displayed in a list of 'cross-entity' fields in another context (like api explorer). I wonder if we should add another key - e.g

is_active
Enabled?</in context title>

description

<title>Membership Type Description</title> Description

Comments

I think this PR is mergeable as it as it does not actually touch the form. The above is a discussion arising from it.

@colemanw colemanw merged commit d425721 into civicrm:master May 14, 2018
@colemanw
Copy link
Member

colemanw commented May 14, 2018

Yes we could add a new key to the xml. I think this would make sense:

<field>
  <name>is_active</name>
  <title>Is Active</title>
  <type>boolean</type>
  <default>1</default>
  <comment>Is this membership_type enabled</comment>
  <add>1.5</add>
  <html>
    <label>Enabled?</label>     <-- New!
    <type>CheckBox</type>
  </html>
</field>

OTOH I see no problem with just updating the title attribute here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants