-
Notifications
You must be signed in to change notification settings - Fork 286
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
Add the Audiences Widget Area happy path view #8138
Comments
Note that I've moved this back to the Backlog pending the approval of the additional source link section in the design doc. Cc @ivonac4 |
The source link aspect has now been finalised, so I've moved this back to AC. |
@techanvil I've created a draft IB here, however, I'm curious why we need the new WidgetArea here. With the title, info notice and source link inside the AudienceTiles component, we could renamed this to AudienceTilesWidget then simply register this as a widget within the existing AREA_MAIN_DASHBOARD_TRAFFIC_PRIMARY widget area. We would achieve the same with less code and keep the dashboard consistent with other modules where each module has it's own single widget area. |
Thanks @benbowler. It's a fair question, however, in my view the new audiences section on the dashboard conceptually fits better with the idea of a widget area - it follows the layout of a widget area with the title, "change groups" CTA, and the discrete widgets which follow the general look of individual widgets. Indeed I had initially envisaged each audience tile would itself be a widget but the design choice to show them in a tabbed view for mobile resulted in the move to the single If we implement the whole thing as a widget we'll effectively be reproducing the look of a widget area in a widget and have to reimplement the title and CTA section for one thing. I don't see it being an issue to have an additional widget area for the Traffic context - if we were only going to have a single area per context, we wouldn't really need widget areas at all as a separate grouping under the widget contexts. Plus the I'd rather stick with the current plan of using a widget area with individual widgets for the components as described in the widgets section in the design doc. The widget area should be responsible for rendering the title and CTA, with each individual widget responsible for determining its own visibility and thus active state. The widget area then doesn't need any top level logic for determining its own active state and it keeps things a bit more modular. Admittedly, as time goes on we might find it could be preferable and simpler to actually consolidate things as you've suggested, but I'd rather stick with the current plan for now and iterate/optimise further down the line if we need to. With that in mind if you are happy taking this approach, please can you update the IB accordingly. Please let me know your thoughts either way. |
I'm un-assigning myself as I'm about to take some holiday. I can pick this up after and address the comments unless someone from Squad 2 picks it up in the mean time. |
Thanks @kuasha420, SGTM! Please note that I've added an additional point to hopefully save a bit of time when it comes to generating the source link URL. IB ✅ |
QA Update
|
Hi @kelvinballoo, regarding your observations:
That's expected, and hence, it was stated in the QAB. Configuring the audience is part of #8131.
Validating the data is not part of this ticket. This ticket is solely responsible for registering (displaying) the widgets on the dashboard. The data logic was implemented in #8136. It applies to the rest of your observations regarding validations. If you encounter any issues, please check with @techanvil and create a new ticket.
If Analytics is not connected, the widgets will not be registered/displayed. Sorry, but this was not explicitly stated in the QAB. |
QA Update ✅Thanks @hussain-t
With that I confirm that this ticket is working as expected: ✅
Moving this ticket to 'Approval' |
Feature Description
Add the Audiences Widget Area to the dashboard in the happy path view. This includes displaying an Audience Tile per selected audience with live data, the Info Notice showing a static message, and the Analytics source link.
See audiences widget area in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
audienceSegmentation
feature flag enabled, and an audience selection defined, the Audiences Widget Area should be displayed on the dashboard.traffic
widget context).Implementation Brief
Implement
Footer
in and maketitle
optionalwidgetAPI.registerWidgetArea
:assets/js/googlesitekit/widgets/datastore/areas.js
:CTA
, take inFooter
as asetting
property inregisterWidgetArea
action.title
ensure it can be undefined.assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.js
:title
, nothing needs to be done as it's already conditionally rendered.googlesitekit-widget-area-footer
cell and render theFooter
component when it is available.assets/sass/widgets/_widget-area.scss
.Inside of
assets/js/googlesitekit/widgets/register-defaults.js
, register a new widget area usingwidgetsAPI.registerWidgetArea
whenaudienceSegmentation
feature flag is enabled with the following properties:AREA_MAIN_DASHBOARD_TRAFFIC_AUDIENCE_SEGMENTATION = "mainDashboardTrafficAudienceSegmentation"
, stored inassets/js/googlesitekit/widgets/default-contexts.js
subtitle
should match the copy in Figma.Footer
should be a new componentAudienceAreaFooter
that displays the Source Link as seen in Figma. Detailed below.priority
should be set to2
to show the widget area below the primary traffic widget area within the traffic widget context.Create
InfoNoticeWidget
inassets/js/modules/analytics-4/components/audience-segmentation/dashboard
folder.:Widget
prop.InfoNotice
component, and for now, pass static textThe higher the portion of new visitors you have, the more your audience is growing. Looking at what content brings them to your site may give you insights how to reach even more people.
ascontent
andGot it
asdismissLabel
.onDismiss
should be no-op for now.whenActive( { moduleName: 'analytics-4' } )
hoc.Rename the
AudienceTiles
component toAudienceTilesWidget
and wrap the export in awhenActive( { moduleName: 'analytics-4' } )
hoc.Inside of
assets/js/modules/analytics-4/index.js
, register the above two widgets usingwidgets.registerWidget
whenaudienceSegmentation
feature flag is enabled with the following properties:AudienceTilesWidget
slug
:analyticsAudienceTiles
priority
:1
wrapWidget
:false
.modules
:[ 'analytics-4' ]
.[ AREA_MAIN_DASHBOARD_TRAFFIC_AUDIENCE_SEGMENTATION ]
InfoNoticeWidget
slug
:analyticsAudienceInfoNotice
priority
:2
Within the
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/
folder, create a new component calledAudienceAreaFooter
which renders the source link using theSourceLink
component.analytics-4
getServiceReportURL()
selector with atype
argument of'audiences'
and the dates supplied as usual.Only render the component if Analytics module is connected, otherwise return
null
.Test Coverage
registerWidgetArea
action.QA Brief
audienceSegmentation
feature flag is enabled using the tester plugin.getAvailableAudiences
selector to get the available audiences.name
property from the output of the above command and run thesetConfiguredAudiences
action with the copied audiences:saveAudienceSettings
action to save the configured audiences:Changelog entry
The text was updated successfully, but these errors were encountered: