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

VerticalTabsTab always fires e.preventDefault(), preventing the use of opening in a new tab via cmd/ctrl + click #6805

Closed
rhamilto opened this issue Jan 17, 2022 · 2 comments · Fixed by #6901

Comments

@rhamilto
Copy link
Member

rhamilto commented Jan 17, 2022

Describe the issue. What is the expected and unexpected behavior?
patternfly-catalog-view-extension's VerticalTabsTab always fires e.preventDefault(), preventing the use of opening in a new tab via cmd/ctrl + click.

Please provide the steps to reproduce. Feel free to link CodeSandbox or another tool.
See OperatorHub in OpenShift console. Try cmd/ctrl + clicking one of the categories on the top left of the page. Note the click does not open the link in a new tab. Changes in openshift/console#10853 enable the opening of one of these links in a new tab via right click > Open Link in New Tab.

Is this a bug or enhancement? If this issue is a bug, is this issue blocking you or is there a work-around?

Bug. Because e.preventDefault() always fires, there is no way to work around the issue. The addition of an optional prop (for optimal backward compatibility) and an update to handleActivate is one possible fix:

  // Check for a modified mouse event. For example - Ctrl + Click
  const isModifiedEvent = (event: React.MouseEvent<HTMLElement>) => {
    return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
  };

  const handleActivate = (e: React.MouseEvent<HTMLElement>) => {
    if (allowModifiedClick) {
      if (isModifiedEvent(e)) return;
    }
    e.preventDefault();
    if (onActivate) {
      onActivate();
    }
  };

What is your product and what release version are you targeting?

OpenShift 4.10.

@gruselhaus
Copy link
Contributor

gruselhaus commented Jan 26, 2022

@jerolimov and I discussed this issue today since we have a similar behavior inside the catalog in the dev console.

@jerolimov suggested a different approach than @rhamilto. We could go with something like we go with at the Button component where you can pass a component prop which will replace the internal <a> tag.

Button component example:

const RouterLink = () => (
  <Button variant="link" component={Link}>
    Router link
  </Button>
)

Suggested solution:

<VerticalTabsTab component={props => <Link {...props} to="#"/>}>
 Children
</VerticalTabsTab>

When we pass a link component, react-router will handle the job for us.

What do you think?

@rhamilto
Copy link
Member Author

No objections to @gruselhaus' alternate approach, but note the other places this bug exists in OpenShift console, we implemented the approach I documented in the original bug report.

gruselhaus added a commit to gruselhaus/patternfly-react that referenced this issue Feb 8, 2022
gruselhaus added a commit to gruselhaus/patternfly-react that referenced this issue Feb 9, 2022
gruselhaus added a commit to gruselhaus/patternfly-react that referenced this issue Feb 9, 2022
nicolethoen pushed a commit that referenced this issue Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants