Skip to content
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

WIP: feat(menu): add link menu item option #867

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wallw-teal
Copy link
Contributor

@wallw-teal wallw-teal commented Feb 6, 2020

Adds a link option to menu items which is either a string (the URL) or an object of {href, target, opener, referrer} such as {href: 'somepage.html', target: '_blank'}. The openener and referrer options are only needed to open windows in the same process, which gives that page access to window.opener and window.opener.document (those should be omitted in almost all cases).

This is all to address various problems with window.open that make it an anti-pattern in JS.

WIP because:

  • ColumnActions is using goog.window.open. Pretty sure this one is replaceable with further investigation.
  • os.openPopup may be a legitimate use of window.open, and I'm unsure how to either make that an exception to a conformance violation or implement that differently
  • Buttons in button bars that are wrapped in <a>...</a> are not styled correctly
  • Menu link text is highlighted via the a:hover style, while none of the other options are not (not sure if bug or feature)

Also, should we indicate that external links are external (a la Wikipedia)? Should we also force those links into a new tab?

Adds a link option to menu items which is either a string (the URL) or
an object of {href, target, opener, referrer} such as {href:
'somepage.html', target: '_blank'}. The openener and referrer options
are only needed to open windows in the same process, which gives that
page access to window.opener and window.opener.document (those should be
omitted for any external links for security reasons).
johnstacy pushed a commit to johnstacy/opensphere that referenced this pull request Mar 10, 2020
…here:THIN-14128 to master

* commit '63965092a7ab135b06a935371a1f8104123d851c':
  feat(theming): fixed colorblind themes when adding an exception theme
  feat(scss): added content fit styling to scale images
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant