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

feat(app): 680 UseTexture composable as component #757

Merged
merged 10 commits into from
Sep 6, 2024

Conversation

JaimeTorrealba
Copy link
Member

closes #680

Convert a composable like this one, as a component is not difficult, what I would like to know is what do you think about this one

Component Name

Vueuse, use the same name of the composable and seems to have no problem (the component start with uppercase), but what do you think? Could this confuse users? Any other alternatives.

useTexture and UseTexture

    <Suspense>
      <UseTexture v-slot="{ map }" :map="path">
        <TresMesh>
          <TresBoxGeometry />
          <TresMeshStandardMaterial :map="map" />
        </TresMesh>
      </UseTexture>
    </Suspense>

Props and result

They share the same name in the example they are :map="map" and the result currently is v-slot="{ map}"

I personally don't see any problem but again I would like to know what do you think?

Docs

I would not do the docs just yet, until your answers. But my idea is to put the docs of this just below the existing composable, any thoughts?

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for tresjs-docs ready!

Name Link
🔨 Latest commit 3eb919e
🔍 Latest deploy log https://app.netlify.com/sites/tresjs-docs/deploys/66daff9a37850600081fb19e
😎 Deploy Preview https://deploy-preview-757--tresjs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@alvarosabu alvarosabu self-assigned this Jul 4, 2024
@alvarosabu alvarosabu added p2-nice-to-have Not breaking anything but nice to have (priority) feature labels Jul 4, 2024
Copy link
Member

@alvarosabu alvarosabu left a comment

Choose a reason for hiding this comment

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

Love the idea but not the execution. We need to be more strict on the standardization of the code base and avoid having 3 different approaches to components.

src/composables/useTexture/component.ts Outdated Show resolved Hide resolved
Copy link

pkg-pr-new bot commented Sep 2, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/@tresjs/core@757

commit: 3eb919e

@alvarosabu alvarosabu self-requested a review September 5, 2024 15:36
@alvarosabu alvarosabu enabled auto-merge (squash) September 5, 2024 15:36
@alvarosabu alvarosabu merged commit f01a897 into main Sep 6, 2024
9 of 10 checks passed
@alvarosabu alvarosabu deleted the feature/680-usetexture-component branch September 6, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature needs-documentation p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

UseTexture as component
2 participants