Skip to content
This repository has been archived by the owner. It is now read-only.

All settings panels report the screen name as settings #3029

Closed
shane-tomlinson opened this issue Sep 3, 2015 · 5 comments
Closed

All settings panels report the screen name as settings #3029

shane-tomlinson opened this issue Sep 3, 2015 · 5 comments
Assignees

Comments

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 3, 2015

["settings.communication-prefs-link.visible.true","screen.settings","settings.newsletter.optin.false","settings.refresh","settings.profile_image_not_shown","screen.settings","screen.settings","screen.settings","screen.settings"]

Not a problem if we banish the settings page from being iframed (#2945), but something I found while testing #3006.

@shane-tomlinson shane-tomlinson changed the title iframe flow: all panels report the screen name as settings iframe flow: all settings panels report the screen name as settings Sep 3, 2015
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Sep 3, 2015

cc @zaach.

@zaach zaach self-assigned this Sep 8, 2015
@rfk rfk added this to the FxA-0: quality milestone Sep 8, 2015
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Sep 8, 2015

@zaach moved to now

@shane-tomlinson shane-tomlinson changed the title iframe flow: all settings panels report the screen name as settings All settings panels report the screen name as settings Sep 8, 2015
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Sep 8, 2015

This is a problem in general, not for just the iframe flow.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 5, 2015

switched to @shane-tomlinson

@zaach
Copy link
Contributor

@zaach zaach commented Oct 6, 2015

Ah, well I'll dump my current thoughts: the issue is that we get the screenName from the URL of the window at the time the view is created, which is a problem when we create multiple views at once, e.g. on /settings. Ideally, views could declare their screenName and we'd prefer that if present.

shane-tomlinson pushed a commit that referenced this issue Oct 9, 2015
There were two problems:
* Subviews always took the screen name of their parent because subviews
  are created with the same options as the parent view, which includes
  the parent view's screen name.
* If the user opened the site directly to a subview, the settings panel
  would take the screen name of the child view, because the settings panel
  took its screen name from the URL, which was the subview's URL.

Add a `screenName` property with the correct name to all settings pages.
If `screenName` is set on the prototype, ignore the screen name passed
in on construction.

fixes #3029
shane-tomlinson pushed a commit that referenced this issue Oct 9, 2015
There were two problems:
* Subviews always took the screen name of their parent because subviews
  are created with the same options as the parent view, which includes
  the parent view's screen name.
* If the user opened the site directly to a subview, the settings panel
  would take the screen name of the child view, because the settings panel
  took its screen name from the URL, which was the subview's URL.

Add a `screenName` property with the correct name to all settings pages.
If `screenName` is set on the prototype, ignore the screen name passed
in on construction.

fixes #3029
shane-tomlinson pushed a commit that referenced this issue Oct 9, 2015
There were two problems:
* Subviews always took the screen name of their parent because subviews
  are created with the same options as the parent view, which includes
  the parent view's screen name.
* If the user opened the site directly to a subview, the settings panel
  would take the screen name of the child view, because the settings panel
  took its screen name from the URL, which was the subview's URL.

Add a `screenName` property with the correct name to all settings pages.
If `screenName` is set on the prototype, ignore the screen name passed
in on construction.

fixes #3029
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants