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

Do not inherit text-decoration on <a> #3352

Closed
wants to merge 3 commits into from

Conversation

sambostock
Copy link

@sambostock sambostock commented Mar 1, 2023

Description

It seems reasonable that by default links should be decorated (underlined), and that undecorated links are the special case, not the other way around.

Closes #837 (tentatively

Checklist:

  • I have performed a self-review of my own code
  • I have added a short summary of my change to the CHANGELOG.md
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

cc. @pngwn & @aliabid94, who originally added this in d6fbc32

It seems reasonable that by default links should be decorated
(underlined), and that undecorated links are the special case, not the
other way around.
With `a { text-decoration: inherit; }` removed from the CSS reset, there
should be no need to explicitly specify these anymore.
@abidlabs abidlabs requested a review from pngwn March 1, 2023 18:05
@gradio-pr-bot
Copy link
Collaborator

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@gradio-pr-bot
Copy link
Collaborator

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-3352-all-demos

@pngwn
Copy link
Member

pngwn commented Mar 1, 2023

I'll try to get to this before the end of the week. I don't have any issue with the code but this could break the UI anywhere we are using an anchor tag for anything, so will need thorough testing.

@pngwn
Copy link
Member

pngwn commented Mar 1, 2023

As an aside, are we sure #837 is still present? The HTML component gets the prose class added (https://github.com/gradio-app/gradio/blob/main/ui/packages/html/src/HTML.svelte#L14) and .prose a has text decoration (https://github.com/gradio-app/gradio/blob/main/ui/packages/theme/src/typography.css#L170-L175)

@abidlabs
Copy link
Member

abidlabs commented Mar 6, 2023

Tested, you're right @pngwn links show up just fine:

import gradio as gr

with gr.Blocks() as demo:
    gr.HTML(
"""
Visit <a href="www.google.com">www.google.com</a> and click <button>here</button>
""")
    
demo.launch()

image

Looks like we can close this PR and the underlying issue (#837). Also tested a related issue (#2520), which I can confirm is still open on the latest version of gradio.

@pngwn
Copy link
Member

pngwn commented Mar 6, 2023

Awesome.

@pngwn pngwn closed this Mar 6, 2023
@sambostock sambostock deleted the underline-links branch March 6, 2023 16:30
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.

HTML Output Hyperlink Styling
4 participants