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

This adds the ability to toggle a field description #4094

Merged
merged 1 commit into from Apr 24, 2015
Merged

This adds the ability to toggle a field description #4094

merged 1 commit into from Apr 24, 2015

Conversation

flashbackzoo
Copy link
Contributor

If a field requires a long description it can be useful have it hidden until the user needs to read it.

Description hidden:
more-info-closed

Description show:
more-info-open

I'm looking for feedback on the technical approach here, so I have only pushed the initial show/hide logic at this stage. There is additional styling required to get the layout looking like the screenshots.

@chillu
Copy link
Member

chillu commented Apr 22, 2015

Interesting! When developing the help text I think we deliberately decided to show it by default. It would be good to make the "info" icons opt-in though (globally and per field), as an easier way to show longer help text.

Please have a read through historical discussions to get context:
#969
#1029
#986
#525
#979

Make sure the styling works correctly with different field layout combos, like in FieldGroup or a DateField with a dmy configuration option (three separate fields). There's a frameworktest module which creates many of these combinations, I've used it for similar sanity checks before: https://github.com/silverstripe-labs/silverstripe-frameworktest

@@ -138,6 +138,8 @@ public static function name_to_label($fieldName) {
* @return string
*/
public static function create_tag($tag, $attributes, $content = null) {
Requirements::javascript(FRAMEWORK_DIR . '/javascript/FieldToggleDescription.js');
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in the FormField() function, which is where all other formfields require their javascript. It should also only be included if necessary.

Alternatively, if you don't want to include this check, then you can put it in one of the existing framework/admin/javascript files; likely LeftAndMain.FieldHelp.js, which already has similar field helpers.

Copy link
Member

Choose a reason for hiding this comment

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

Also make sure that's included in the main Requirements::combine_files() call in CMSMain, we don't want to cause more asset loads to slow down the CMS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll move it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if it's in LeftAndMain we should remove it from here. This is a CMS specific feature not a field specific one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, yup that was an oversight on my part, i forgot to remove it.

@flashbackzoo
Copy link
Contributor Author

The naming convention is now more consistent with FieldHelp / Tooltip. And there are some docs. @chillu @tractorcow what are your thoughts on this approach?

@flashbackzoo
Copy link
Contributor Author

The initial styling is there now. Here's a screenshot of the styles applied in the files area.
file-upload

There are a lot of image file changes as a result of adding an icon to the buttons sprite (admin/images/btn-icon/information.png) and running a compass compile. I've had a good look around the CMS and everything seems to be presenting fine.

I'm going to do some testing on other field types as @chillu suggested, and as a result probably push up another commit :)

@flashbackzoo
Copy link
Contributor Author

Here are some screenshots of various field types.

toggle-desc-checkboxes
toggle-desc-dropdown
toggle-desc-html
toggle-desc-text
toggle-desc-textarea
toggle-desc-upload

@chillu @tractorcow If you're happy with this, I'll squash the commits...

@jonom
Copy link
Contributor

jonom commented Apr 23, 2015

Hey @flashbackzoo you may have already tested but just wondering if this works for a

DateField with a dmy configuration option (three separate fields)

as chillu pointed out, and also on individual fields nested within a FieldGroup?

@flashbackzoo
Copy link
Contributor Author

Hey @jonom - Yup it works on DateField with three fields.

For a FieldGroup you can add a toggleable description to the group.

The CheckboxField template has no middleColumn, so it doesn't work there. I think that's more of a CheckboxField issue though. If the CheckboxField template was updated to have a middleColumn, consistent with other CMS fields, it will work.

tractorcow pushed a commit that referenced this pull request Apr 24, 2015
This adds the ability to toggle a field description
@tractorcow tractorcow merged commit 6c069ce into silverstripe:3 Apr 24, 2015
@tractorcow
Copy link
Contributor

Thanks @flashbackzoo :D

@flashbackzoo flashbackzoo deleted the pulls/toggle-description branch April 27, 2015 20:55
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.

None yet

5 participants