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

Add anchor tag for hash link #469

Merged
merged 5 commits into from Feb 8, 2023
Merged

Add anchor tag for hash link #469

merged 5 commits into from Feb 8, 2023

Conversation

PeterYusuke
Copy link
Contributor

PR for issue #416.

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

After these steps, you're ready to open a pull request.

Added anchor tag that created with key property at any component.
This allows us to jump to to a specific place using hash in URL like "destination/url/#target".

Usage:

    pc.box(
        pc.text("This is target"),
        key="target",
    )

Above code creates this.
Anchor tag is placed at the first child tag.

    <Box key="target">
        <a name="target" id="target"></a>
        <Text>{`This is target`}</Text>
    </Box>

@Lendemor
Copy link
Collaborator

Lendemor commented Feb 7, 2023

Is it not possible to add the id directly to the <Box> tag in this case?

@PeterYusuke
Copy link
Contributor Author

Is it not possible to add the id directly to the <Box> tag in this case?

I think it's possible. Should I change this?

@Alek99 Alek99 self-requested a review February 8, 2023 05:54
@Alek99
Copy link
Contributor

Alek99 commented Feb 8, 2023

Is it not possible to add the id directly to the <Box> tag in this case?

I think it's possible. Should I change this?

Yeah it would simplify the compiled code a bit, can you try this and see if it works. Thanks for the contribution

@PeterYusuke
Copy link
Contributor Author

I changed to add id directly to component.
This works as hash link.

Usage :

    pc.box(
        id="target",
        pc.text("some text"),
    )

Above code creates this js code.

    <Box id="target">
        <Text>{`some text`}</Text>
    </Box>

Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good calling adding this id field . I'm a bit confused though - looks like the Anchor class got reverted? Don't we need that?

Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm, tried this locally and it works great - awesome work!

@picklelo picklelo linked an issue Feb 8, 2023 that may be closed by this pull request
@Lendemor
Copy link
Collaborator

Lendemor commented Feb 8, 2023

Good calling adding this id field . I'm a bit confused though - looks like the Anchor class got reverted? Don't we need that?

I think it was just there as a support for the id prop, but the Anchor itself didn't have a purpose.
Adding the id directly on the component work the same.

@picklelo picklelo merged commit 60949e2 into reflex-dev:main Feb 8, 2023
ACucos1 pushed a commit to ACucos1/rd-pynecone that referenced this pull request Feb 28, 2023
@mister-rao
Copy link

was looking for anchor hash navigation. Adding id to components work, but is there any way to make the scrolling smooth ?

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.

Add anchor tag component
5 participants