chop-dbhi / cilantro

Official client for Harvest (http://harvest.research.chop.edu)
http://cilantro.harvest.io
Other
28 stars 8 forks source link

Added option to either display the field on lower case or as it was defined #790

Closed rennomarcus closed 1 year ago

rennomarcus commented 6 years ago

Context: We had a request to change one the modules behavior: vocab. Vocab has a component, called bucket and within this component the name of the field that was selected is displayed. The thing is that one the fields contains a name, Fyler, thus it should not be lower cased.

Solution: I added an option to let the field not be lower cased.

rennomarcus commented 6 years ago

Hi, @bruth . I made this small change. I was hoping you could review and share your thoughts about this change.

Thanks!

bruth commented 6 years ago

A more general solution could be to provide a function in the config that takes the names as an argument and then call that in the component if defined.

{
  // ..

  fieldNameDisplay: function(names) {
    return {
      single: names.single.toLowerCase(),
      plural: names.plural.toLowerCase()
    }
  },
}
rennomarcus commented 6 years ago

Thanks for reviewing it! I agree with what you said and I think it's better this way. I changed the code and I hope it's closer to what you said. Now one can change the function in the config to execute whatever they want with certain names. Example:

fieldNameDisplay: function fieldNameDisplay(names) {
    if (names.single.toLowerCase() !== 'fyler diagnosis') {
        return {
          single: names.single.toLowerCase() || '',
          plural: names.plural.toLowerCase() || ''
        };
    }
    return names;
}
rennomarcus commented 6 years ago

Hi, @bruth. I've updated the comments and removed the function name to maintain the consistency. Let me know if you have any other comments. Thanks!