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

Showcase .gts - Shw::Autoscrollable #2426

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

didoo
Copy link
Contributor

@didoo didoo commented Sep 17, 2024

🛠️ Detailed description

In this PR I have:

  • moved the Shw::Autoscrollable template code under the unified .gts file
  • updated the import path in the template-registry.ts file to include the index file name (as did in previous PR)

Preview: https://hds-showcase-git-showcase-gts-autoscrollable-hashicorp.vercel.app/components/rich-tooltip#collision-detection

🔗 External links

Jira ticket: https://hashicorp.atlassian.net/browse/HDS-3829


👀 Component checklist

  • Percy was checked for any visual regression

💬 Please consider using conventional comments when reviewing this PR.

@didoo didoo requested review from aklkv and a team September 17, 2024 19:02
Copy link

vercel bot commented Sep 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Sep 17, 2024 7:06pm
hds-website ✅ Ready (Inspect) Visit Preview Sep 17, 2024 7:06pm

<div
class="shw-autoscrollable__container"
...attributes
{{this.autoscroll}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I tried to move the autoscroll modifier definition outside of the class/Component declaration, but it didn't work: for some reasons that I don't understand only the last instance actually called the centerScrollableArea function.

If you know why and want to give it a go and try do it "properly" feel free to push directly in the branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did attempt to do the same but gave up as this modifier is closely tide to this components. It might be good to follow up as a stand alone PR in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants