Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Spread extra props into components for event tracking. #2376

Closed
1 task
ghost opened this issue Oct 11, 2019 · 1 comment · Fixed by #2377
Closed
1 task

Spread extra props into components for event tracking. #2376

ghost opened this issue Oct 11, 2019 · 1 comment · Fixed by #2377
Labels
ws-home Tasks for the WS Home Team

Comments

@ghost
Copy link

ghost commented Oct 11, 2019

Is your feature request related to a problem? Please describe.

Rather than having custom logic in psammead for simorgh event tracking, can we instead spread props into this component, and set the data attribute in simorgh please? IMO we shouldnt have custom logic specific to simorgh when its not needed. Someone else using Brand may not do event tracking like we are. It also makes it very difficult for us to change our event tracking method in the future.

Describe the solution you'd like
Revert work done on

Psammead component:

export const IndexAlsos = props => (
    <div {...props}>
      {props.children}
    </div>
  );

Simorgh:

  return (
    <IndexAlsos data-e2e="index-alsos">{...}</IndexAlsos>

Describe alternatives you've considered

Testing notes
[Tester to complete]

Dev insight: Will there be any potential regression? etc

  • This feature is expected to need manual testing.

Additional context
Add any other context or screenshots about the feature request here.

@ghost ghost added the ws-home Tasks for the WS Home Team label Oct 11, 2019
@ghost ghost added this to the Event Tracking (WS FP) milestone Oct 11, 2019
@ghost ghost self-assigned this Oct 11, 2019
@ghost ghost changed the title Spread extra prop into component for event tracking. Spread extra props into component for event tracking. Oct 11, 2019
@ghost ghost mentioned this issue Oct 11, 2019
3 tasks
@ghost ghost changed the title Spread extra props into component for event tracking. Spread extra props into components for event tracking. Oct 11, 2019
@jamesdonoh
Copy link
Contributor

jamesdonoh commented Oct 11, 2019

My concern about the above example was just that surely I can pass any props I like to IndexAlsos and they'll end up on the (here unstyled) div, e.g.

<IndexAlsos data-e2e="index-alsos" potato="yes">

I see styled components swallow these but are we sure we are only ever going to spread to them?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant