Add location tag to download button

RESOLVED FIXED

Status

www.mozilla.org
Analytics
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: pgerman, Assigned: agibson)

Tracking

Production

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
To the download button please add the following data attribute:

data-download-location = "<location>" //acceptable values include 'nav', 'primary cta', and 'other'
(Assignee)

Comment 1

6 months ago
(In reply to pgerman from comment #0)
> To the download button please add the following data attribute:
> 
> data-download-location = "<location>" //acceptable values include 'nav',
> 'primary cta', and 'other'

Peter, 

We can easily add values for this to common elements that appear on multiple pages (e.g. global nav etc), but do you have a list of pages that you would like to target here with other values?

One approach we could take is to use a default value of 'primary cta' for all buttons, and then pass in exceptions for a list of known pages / placements. Does that sound like a feasible approach?
Flags: needinfo?(peter.german.bugs)
(Reporter)

Comment 2

6 months ago
Can we talk about a solution tomorrow towards the start of the day EST?
(Assignee)

Updated

6 months ago
Assignee: nobody → agibson
Status: NEW → ASSIGNED
(Assignee)

Comment 3

6 months ago
After talking this through with Peter today we've aligned on the following approach:

- Add an optional attribute to the download button helper that can pass in a value.
- Values for the attribute will be either 'nav', 'sub nav', 'primary cta' or 'other'.
- There will be no default value for this attribute, it will only be included on a button if defined explicitly in the template.
- Add attributes to buttons on all new style hub pages. We're not as concerned with tracking legacy pages.
(Assignee)

Updated

6 months ago
Flags: needinfo?(peter.german.bugs)
(Assignee)

Comment 4

6 months ago
Peter, I pushed a patch for this to demo. Can you please test on all new hub style pages?

https://www-demo3.allizom.org/en-US/firefox/
Flags: needinfo?(pgerman)
(Assignee)

Comment 5

6 months ago
Created attachment 8877583 [details] [review]
GitHub pull request
(Reporter)

Comment 6

6 months ago
Hi Alex,

Is it possible to do this on /new/ as well even though it uses the old style?

Thanks,

PG
Flags: needinfo?(pgerman)
(Reporter)

Comment 7

6 months ago
Can we also put this on the google play / iTunes buttons?
Flags: needinfo?(agibson)
(Assignee)

Comment 8

6 months ago
Answer should be yes to both questions - will update tomorrow, thanks.
Flags: needinfo?(agibson)
(Assignee)

Comment 9

6 months ago
(In reply to Alex Gibson [:agibson] from comment #8)
> Answer should be yes to both questions - will update tomorrow, thanks.

Peter, demo should now be updated for both /new/ and mobile buttons. Can you test again? Thanks

https://www-demo3.allizom.org/en-US/firefox/new/
https://www-demo3.allizom.org/en-US/firefox/android/
https://www-demo3.allizom.org/en-US/firefox/ios/
Flags: needinfo?(pgerman)
(Reporter)

Comment 10

6 months ago
Looks good. 

Not sure if this is scope creep, but I didn't see it on scene 2. Should I make a new bug for that?
Flags: needinfo?(pgerman) → needinfo?(agibson)
(Assignee)

Comment 11

6 months ago
(In reply to pgerman from comment #10)
> Looks good. 
> 
> Not sure if this is scope creep, but I didn't see it on scene 2. Should I
> make a new bug for that?

Why do you need it on scene2 out of curiosity? There's only one button on there and it's visually hidden
Flags: needinfo?(agibson) → needinfo?(pgerman)
(Reporter)

Comment 12

6 months ago
There are the mobile download buttons, no?
Flags: needinfo?(pgerman) → needinfo?(agibson)
(Assignee)

Comment 13

6 months ago
(In reply to pgerman from comment #12)
> There are the mobile download buttons, no?

Oh crap, yes you are right. These should have a value of 'other'?
Flags: needinfo?(agibson) → needinfo?(pgerman)
(Reporter)

Comment 14

6 months ago
yeah. Thanks.
Flags: needinfo?(pgerman) → needinfo?(agibson)
(Assignee)

Comment 15

6 months ago
(In reply to pgerman from comment #14)
> yeah. Thanks.

Updated mobile buttons on scene2, also added attributes to the buttons on the new /firefox/focus/ page

https://www-demo3.allizom.org/en-US/firefox/new/?scene=2
https://www-demo3.allizom.org/en-US/firefox/focus/
Flags: needinfo?(agibson) → needinfo?(pgerman)
(Reporter)

Comment 16

6 months ago
Thanks. Publish when ready.
Flags: needinfo?(pgerman)

Comment 17

6 months ago
Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/3625a597f0c7fb65238b7b4d28c49f7a5f06e5b8
[fix bug 1359982] Add location tag to download button

https://github.com/mozilla/bedrock/commit/218d83f1f235b84dee21e29cfa70a93d2f5c2b72
Merge pull request #4903 from alexgibson/bug-1359982-download-button-location-tag

[fix bug 1359982] Add location tag to download button

Updated

6 months ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.