zurb / foundation-apps

The first front-end framework created for developing fully responsive web apps.
http://foundation.zurb.com/apps
MIT License
1.58k stars 216 forks source link

Standardizing active state tracking across components #740

Closed soumak77 closed 8 years ago

soumak77 commented 8 years ago

The way in which the active state of a component is tracked needs to be standardized across components. The change for PR #670 is great, however, it also allows for the active state to be changed outside of the directive due to the two-way binding. Components could be updated to watch for changes to this two-way bound value, however, the current model for activating/deactivating components is already established with the foundation sub/pub system.

Keeping the foundation sub/pub system as the standard, the components can be updated to send messages when the active state changes:

The changes made with the zf-advise directive where intended to provide the ability to track the active state of the components, but really just track the state of the animation on the component. This has caused some intermittent bugs to pop up in my app due to thinking they were the same. I propose the messages sent via the animateAndAdvise method be renamed to specify they are related to animations and not related to the active state of the component. The zf-advise directive could be reserved for when a user needs to subscribe to animation events on the component. The following animation states can be tracked:

* Messages should use show/hide or open/close based on F4A standard

soumak77 commented 8 years ago

@zurbchris @gakimball Let me know your thoughts on this. I'll be making these changes in my fork. I can get a PR submitted today if you provide feedback.

zurbchris commented 8 years ago

makes sense to me.

soumak77 commented 8 years ago

I know both show/hide and open/close are used in messages, but which is the preferred standard so the animation messages can use those values?

soumak77 commented 8 years ago

I made this comment on #670, but should include it here for completeness:

Accordions and Tabs do not use the foundation sub/pub system to handle changing active state of elements. In these cases, two-way binding of scope.active would have to be used to be notified of active state changes. The two-way bound value should be readonly until support is added to the Accordion and Tabs to support changing to active state outside of the directive.

soumak77 commented 8 years ago

this has been completed in https://github.com/base-apps/angular-base-apps