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

[WebProfilerBundle] Fix cursor on link that has no href #50376

Merged
merged 1 commit into from May 22, 2023

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented May 20, 2023

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
License MIT

A a link tag with no href has the wrong cursor icon by default when hovering it. Eg: https://codepen.io/philetaylor/pen/mdzQoxR

Before

before.mov
anotherbefore.mov

After

after.mov

@carsonbot carsonbot added this to the 6.3 milestone May 20, 2023
@nicolas-grekas nicolas-grekas changed the title [Web-profiler-bundle] Fix cursor on link that has no href [WebProfilerBundle] Fix cursor on link that has no href May 22, 2023
@nicolas-grekas
Copy link
Member

Won't this break CSP rules?

@PhilETaylor
Copy link
Contributor Author

It doesn't appear to.

@nicolas-grekas
Copy link
Member

Thank you @PhilETaylor.

@stof
Copy link
Member

stof commented May 22, 2023

The right markup for this would be a <button type="button"> (with the appropriate CSS to make it look like a textual link). A <a> without href is neither interactive nor focusable, so this would be an accessibility issue.

@nicolas-grekas there is no CSP at all on profiler pages (our toolbar has to avoid CSP issues as it gets injected into the project pages)

@PhilETaylor
Copy link
Contributor Author

The right markup for this would be a

I did type that out, but deleted it, sorry, was in the car earlier. I guess we need to revert this one, or revert it with a new PR, where would the css go?

@PhilETaylor
Copy link
Contributor Author

As a button, with no css, it also doesn't have a pointer.

Screen.Recording.2023-05-22.at.15.06.05.mov

@PhilETaylor
Copy link
Contributor Author

Markup:
line <button type="button" class="text-small sf-toggle sf-toggle-button" data-toggle-selector="#sf-trace-{{ loop.index0 }}">{{ caller.line }}</button> (<button type="button" class="text-small sf-toggle sf-toggle-button" data-toggle-selector="#sf-context-{{ loop.index0 }}">context</button>):

With new css in profiler.css.twig

.sf-toggle-button {
    background: none!important;
    border: none;
    padding: 0!important;
    color: rgb(29, 78, 216);
    cursor: pointer;
}

Looks like this - if this is ok I can PR this:

Screen.Recording.2023-05-22.at.15.10.28.mov

ok?

@PhilETaylor
Copy link
Contributor Author

converted to a button in PR #50386

@fabpot fabpot mentioned this pull request May 22, 2023
nicolas-grekas added a commit that referenced this pull request May 23, 2023
This PR was merged into the 6.3 branch.

Discussion
----------

Fix cursor on link that has no href

[WebProfilerBundle] Fix cursor on link that has no href

link to button

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->

Alternative approach for #50376

`@stof` `@nicolas`-grekas

Commits
-------

893d299 Convert A link to Button
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants