[FEATURE] Add willBecomeVisible & willBecomeHidden event hooks

[elwayman02]

This allows developers to hook into the visibility change before it happens, to mirror the becameVisible and becameHidden hooks that already exist.

Also refactors notifyBecameVisible and notifyBecameHidden into a single method that takes an event name to trigger on the view and its visible children

[elwayman02]

All 44 Failing tests exist in master

[mixonic]

I'm extremely hesitant to add new APIs around visibility toggling. It seems like the kind of thing Ember should be doing less of, and not building a richer ecosystem around. Addons would be great for features like toggling visibility.

[elwayman02]

If there is an effort to extract visibility toggling into an addon I am happy to move this PR there; I don't have the bandwidth to spearhead such an effort myself. However, if an effort is not already underway and you plan to push for it to happen, I think it makes sense for the functionality
that will be extracted to at least have parity in its current implementation, which is not the case today. This PR solves that problem, wherever it ends up living a month from now.

This is as much a bugfix as a feature as far as I'm concerned...because the current code is only a half-implementation of what is necessary.

[mixonic]

If there is an effort to extract visibility toggling into an addon I am happy to move this PR there;

There isn't yet. But if this is a feature we want to remove, I don't think adding new APIs for it is the right direction to head.

The failures look like JSCS failures fwiw. I just went down a rabbit hole thinking they were from master, but upon coming back out of the hole I don't believe they are on master. Master has some saucelabs failures, that's all.

[rwjblue]

The only test failures here are from You must supply@public,@Private, or@Protectedfor block comments. due to the type of comments that you have added.

Change the following:

/**
* Toggle visibility of the view, firing event hooks before and after the element visibility is changed
* @private
*/

To

/**
  Toggle visibility of the view, firing event hooks before and after the element visibility is changed
  @private
*/

[elwayman02]

Hmm ok, thanks @rwjblue. I don't know why I see the same number of test failures on master, then...

[elwayman02]

@mixonic: Ultimately, it's additive and when an addon is built to extract this functionality, this change can come with it. I agree with your sentiment, but unless there is an ongoing plan to execute this extraction now, I don't see the point in holding off on this PR. Better to consolidate this functionality in one place and extract it altogether when that eventually takes place, than to leave it out and then have to remember to go find this closed PR and bring it into the addon at some unknown point in the future.

[stefanpenner]

There isn't yet. But if this is a feature we want to remove, I don't think adding new APIs for it is the right direction to head.

yes, visibility concept is from the old sproutcore days... With templates + css this can exist, without all the extra machinery and quirkyness. I suspect it will be entirely gone in 3.0

[krisselden]

I'm not even sure isVisible is something even being considered for angle-bracket/glimmer components, so I expect it to be gone by 3.0.

[locks]

Can we get an update on this PR?

[homu]

☔ The latest upstream changes (presumably #13658) made this pull request unmergeable. Please resolve the merge conflicts.

[kanongil]

I need to implement custom isVisible behavior in one of my components, which is currently impossible. Unfortunately, this patch won't help either.

I would much prefer that the property is simply deprecated, and the existing implementation not touched until it has been removed in 3.0. This way I can use the existing private hooks to create my custom behavior.

[Serabe]

This has been opened for a while and most core members that commented here seem clearly not in favor of merging this. If it is so, can we get this closed?

Thank you!

[rwjblue]

Ya, closing this for now.