Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(nuxt): add setLayout utility #6826

Merged
merged 39 commits into from
Aug 26, 2022
Merged

Conversation

516310460
Copy link
Contributor

@516310460 516310460 commented Aug 22, 2022

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Aug 22, 2022

βœ… Deploy Preview for nuxt3-docs ready!

Name Link
πŸ”¨ Latest commit 33a7ad6
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6308a5732c689600087574f4
😎 Deploy Preview https://deploy-preview-6826--nuxt3-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 settings.

@516310460
Copy link
Contributor Author

Add useLayout composables. can change layout
Within your pages, components, and plugins you can use useLayout to get access to data that resolves asynchronously.

Added document and composables

nuxt/nuxt#14658

@516310460
Copy link
Contributor Author

@pi0
Copy link
Member

pi0 commented Aug 22, 2022

Thanks for PR. I personally like it if renamed to setLayout. @danielroe What do you think about such utility?

@pi0 pi0 changed the title Add useLayout function to help users to switch layout feat: add useLayout utility Aug 22, 2022
@pi0 pi0 changed the title feat: add useLayout utility feat(nuxt): add useLayout utility Aug 22, 2022
@516310460
Copy link
Contributor Author

Thanks for PR. I personally like it if renamed to setLayout. @danielroe What do you think about such utility?

Currently, it can. Maybe it will have more missions in the future

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

I agree, we don't want to have useLayout(layoutName: string) as the API. (If we used useLayout we would want it to return a ref to the layout, I think.)

setLayout does make sense, and I think preserves the opportunity to add more helpers if needed. We would need to document that this will work on the client, or (if on server) must be used within nuxt route middleware, and, moreover, persisted via a useState so that the client can ensure that the layout it renders is the same.

Possibly we could also implement this workflow - when !nuxtApp.isHydrating? - within this PR.

See https://github.com/nuxt/framework/issues/6824#issuecomment-1222604779.

@516310460
Copy link
Contributor Author

Yes. Named setLayout is consistent with nuxt2, someone may want to judge and set layout in middleware

@danielroe
Copy link
Member

@516310460 FYI, I'll be looking at this shortly to see if it's possible to implement my comment above in this PR.

@516310460
Copy link
Contributor Author

@516310460 FYI, I'll be looking at this shortly to see if it's possible to implement my comment above in this PR.

Yes, I updated the source code to automatically trigger this PR

@516310460 516310460 closed this Aug 23, 2022
@danielroe
Copy link
Member

Is there a reason you closed this?

@516310460
Copy link
Contributor Author

516310460 commented Aug 23, 2022

Is there a reason you closed this?

I need to submit first :)
Link: #6870

This PR, after waiting for development and testing to complete, I'm reopening

@danielroe
Copy link
Member

Oh, I see. You can also use multiple branches in your fork for different PRs. (Might be easier.)

@516310460
Copy link
Contributor Author

Oh, I see. You can also use multiple branches in your fork for different PRs. (Might be easier.)

Thanks, I've never done this before, I'll try this in the future

@danielroe danielroe changed the title feat(nuxt): add useLayout utility feat(nuxt): add setLayout utility Aug 25, 2022
@danielroe danielroe requested a review from pi0 August 25, 2022 11:39
@516310460
Copy link
Contributor Author

@danielroe Thanks you. source context

@danielroe
Copy link
Member

My to-do list before merging:

  • rename to setPageLayout to make it clearer how this composable operates (wdyt @pi0?)
  • update more docs to refer to this composable
  • fix behaviour in plugins to use router hook as well

@pi0 pi0 added the pending label Aug 26, 2022
@pi0
Copy link
Member

pi0 commented Aug 26, 2022

Makes sense to call it setPageLayout and also improve plugin behavior. I think we can make _layout a permanent state as well using a pages module plugin instead of setState.

@516310460 Since this PR is made from your main branch I think we could move it to a Nuxt branch and continue updating it. Thanks for the idea and initial works ❀️

@danielroe Do you mind if I move forward with branch creation to merge and letting you opening new PR from branch once could?

@pi0
Copy link
Member

pi0 commented Aug 26, 2022

@516310460 Also do you have a discord account? I would like to talk with you with daniel and helping on contribution process together.

@516310460
Copy link
Contributor Author

@516310460 Also do you have a discord account? I would like to talk with you with daniel and helping on contribution process together.

I have an account number

@pi0
Copy link
Member

pi0 commented Aug 26, 2022

Perfect. Mind to send a DM to me? pi0#6582

@danielroe
Copy link
Member

@pi0 Thanks, yes please!

@516310460
Copy link
Contributor Author

516310460 commented Aug 26, 2022

Perfect. Mind to send a DM to me? pi0#6582

Good Please wait a moment. I was having dinner

@516310460
Copy link
Contributor Author

@pi0 I have already DM you

@pi0 pi0 changed the base branch from main to feat/set-page-layout August 26, 2022 14:15
@pi0 pi0 merged commit cad564c into nuxt:feat/set-page-layout Aug 26, 2022
@pi0
Copy link
Member

pi0 commented Aug 26, 2022

~> nuxt/nuxt#14710

danielroe pushed a commit that referenced this pull request Aug 30, 2022
pi0 pushed a commit that referenced this pull request Aug 31, 2022
Co-authored-by: HomWang <516310460@qq.com>
@pi0 pi0 mentioned this pull request Sep 1, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants