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

Analytics: add reportWhen field for visibility #8977

Open
zhouyx opened this Issue Apr 26, 2017 · 10 comments

Comments

@zhouyx
Collaborator

zhouyx commented Apr 26, 2017

follow up of #8315

something like

{
  "on": "visibile",
  "visibilitySpec": {
     ...
    "reportWhen": "hidden"
  },
}

"reportWhen" : "hidden" would equals "on": "hidden" now.
several potential values we can support are hidden detach unlayout

cc @dvoytenko @lannka

@lannka

This comment has been minimized.

Show comment
Hide comment
@lannka

lannka Apr 28, 2017

Collaborator

Can I suggest "hidden" -> "pageHidden". Since this is a trigger for element visibility, people might think "hidden" is "element hidden".

Collaborator

lannka commented Apr 28, 2017

Can I suggest "hidden" -> "pageHidden". Since this is a trigger for element visibility, people might think "hidden" is "element hidden".

@dvoytenko

This comment has been minimized.

Show comment
Hide comment
@dvoytenko

dvoytenko Apr 28, 2017

Collaborator

I agree. Let's, as a first step, add page-hidden or pagehidden event that will alias to hidden and we'll deprecate hidden, update docs, etc.

Collaborator

dvoytenko commented Apr 28, 2017

I agree. Let's, as a first step, add page-hidden or pagehidden event that will alias to hidden and we'll deprecate hidden, update docs, etc.

@jridgewell

This comment has been minimized.

Show comment
Hide comment
@jridgewell

jridgewell Apr 28, 2017

Member

We already have pageHidden in Ads, should we keep the casing the same?

Member

jridgewell commented Apr 28, 2017

We already have pageHidden in Ads, should we keep the casing the same?

@dvoytenko

This comment has been minimized.

Show comment
Hide comment
@dvoytenko

dvoytenko Apr 28, 2017

Collaborator

Cool. RE: casing, though, it looks like currently event casing is dash-based, or all lowercase. Have we seen camel-case events anywhere?

Collaborator

dvoytenko commented Apr 28, 2017

Cool. RE: casing, though, it looks like currently event casing is dash-based, or all lowercase. Have we seen camel-case events anywhere?

@rudygalfi rudygalfi moved this from Sprint Candidate to In Progress This Sprint in Analytics May 1, 2017

@zhouyx zhouyx modified the milestones: Sprint H2 May, Sprint H1 May May 1, 2017

@rudygalfi rudygalfi moved this from In Progress This Sprint to On Deck for Next Sprint in Analytics May 16, 2017

@rudygalfi rudygalfi moved this from On Deck for Next Sprint to In Progress This Sprint in Analytics May 22, 2017

@zhouyx zhouyx modified the milestones: Prioritized FRs, Sprint H2 May May 22, 2017

@rudygalfi rudygalfi moved this from In Progress This Sprint to On Deck for Next Sprint in Analytics Jun 1, 2017

@rudygalfi rudygalfi moved this from On Deck for Next Sprint to Feature Backlog in Analytics Jun 1, 2017

@rudygalfi

This comment has been minimized.

Show comment
Hide comment
@rudygalfi

rudygalfi Sep 27, 2017

Contributor

Can anyone remind me of the status of this?

Contributor

rudygalfi commented Sep 27, 2017

Can anyone remind me of the status of this?

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Sep 28, 2017

Collaborator

Hmmm, I think this has been deprioritized.

Collaborator

zhouyx commented Sep 28, 2017

Hmmm, I think this has been deprioritized.

@dvoytenko

This comment has been minimized.

Show comment
Hide comment
@dvoytenko

dvoytenko Sep 28, 2017

Collaborator

I believe we decided to adopt reportWhen internally, but to keep the old configuration ("hidden").

Collaborator

dvoytenko commented Sep 28, 2017

I believe we decided to adopt reportWhen internally, but to keep the old configuration ("hidden").

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Sep 28, 2017

Collaborator

That's what we already have. I though the originally plan was to expose the reportWhen externally and support more keywords other than 'hidden'. ('unlayout' for example).

Collaborator

zhouyx commented Sep 28, 2017

That's what we already have. I though the originally plan was to expose the reportWhen externally and support more keywords other than 'hidden'. ('unlayout' for example).

@ampprojectbot

This comment has been minimized.

Show comment
Hide comment
@ampprojectbot

ampprojectbot Jan 1, 2018

Collaborator

This issue hasn't been updated in awhile. @zhouyx Do you have any updates?

Collaborator

ampprojectbot commented Jan 1, 2018

This issue hasn't been updated in awhile. @zhouyx Do you have any updates?

@zhouyx

This comment has been minimized.

Show comment
Hide comment
@zhouyx

zhouyx Jul 16, 2018

Collaborator

It's a nice to have feature, but currently there's no request for this feature. Will not prioritize. Labeled it P3, and it's also a good first issue for new contributors.

Collaborator

zhouyx commented Jul 16, 2018

It's a nice to have feature, but currently there's no request for this feature. Will not prioritize. Labeled it P3, and it's also a good first issue for new contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment