988151 - getConfiguration("availableTargets") should take visibility and/or position into account

Status: NEW

(Firefox :: Tours, defect)

Description

[Matthew N. [:MattN]]

I guess we need to define better what "available" means or perhaps include a boolean value indicating visibility.

Use cases:
A1) Page wants to know that if the addons button is in a the *hidden* bookmarks toolbar.
A2) Page wants to know if "accountStatus" is actually visible in the menu panel or if its hidden. (It can currently use getConfigutation("sync") but it's still somewhat weird to say the target is available when the (widget of the) target is @hidden=true (see the panel cavest below).
B) The page is handling showing and hiding the menu itself (perhaps because it wants to show the menu in a tour step before annotating anything inside). The page needs to know if the target is in the panel menu or not in order to know whether hideMenu should be called.

I have been wondering if getTarget or the target's query function could handle some of this itself. 

The menu panel is somewhat special since the items aren't visible before the panel is first constructed and the panel will automatically open to make it visible. We can check the scrollWidth/scrollHeight to take display: none of a parent element into account or handle it in the target's query function before it looks at the anonymous children?

We should probably address these issues in some way or the menu panel is going to end up open more than necessary in the tour once users start customizing (e.g. opening the tour from the Help menu).

Comment 1

[Alex Gibson [:agibson]]

Matt, is there any scope to get this done in time for 29? Even if we don't use it in the tour for release, it would be a nice-to-have should we get breakage over time.

I understand if this is not possible in the time we have left, but thought I'd ask!

Comment 2

[Matthew N. [:MattN]]

It's very unlikely to make 29 since today was the last day for speculative fixes and there are still quite a few bugs marked with higher priority.

I've nominated this for our teams backlog so maybe someone else can pick it up sooner. We can try aim for 30 since we'll still be running the welcome (firstrun) tour.

Comment 3

[Alex Gibson [:agibson]]

Ok understood thanks, Matt!

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

I see it in the backlog and also as 'nice to have' - but there's no obvious reason why this should be a tracking issue.  If there's a speculative fix that's Beta-safe in the next two weeks, please nominate for uplift and we can try it out on the larger population to make sure there aren't any obvious regressions.  No need to drive on this bug though as it's an enhancement more than a regression fix.

Comment 5

[Alex Gibson [:agibson]]

It's worth noting here: by the time this bug gets worked on we will likely be using the current implementation in production. Whatever alterations are made, we need to try and avoid introducing breaking changes if at all possible.

Comment 6

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

Getting this on the radar for the /whatsnew tour deploying in 36. It's in our current wireframes to highlight the icon for users who have it in their toolbar.

Comment 7

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

Removing this as a blocker for 36 as our wireframes work without it.

Comment 8

[Marco Mucci [:MarcoM]]

Discussed during the team's Release 45 Planning meeting.