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

Allow to prefill the login form #6364

Merged
merged 3 commits into from
Mar 24, 2024

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Mar 24, 2024

Main use case for me is to make it easier to log in to the Demokit. We could also use it in the Sandbox.


This PR …

Enhancements

  • The login form can now be prefilled for testing use cases with the new value prop of the k-login-view component

Breaking changes

None

Docs

Probably too obscure to document

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@lukasbestle lukasbestle added the type: enhancement ✨ Suggests an enhancement; improves Kirby label Mar 24, 2024
@lukasbestle lukasbestle added this to the 4.2.0 milestone Mar 24, 2024
@lukasbestle lukasbestle requested a review from a team March 24, 2024 11:51
@lukasbestle lukasbestle self-assigned this Mar 24, 2024
@distantnative
Copy link
Member

I'd suggest to just calling them email and password.

And let's avoid v-bind=$propd - that'll cause issues down the road with Vue 3. already looking into how to reduce this in our existing code (so totally get why you added it as we currently use it a lot).

@lukasbestle
Copy link
Member Author

Got rid of v-bind.

Naming the props just email and password would be super confusing IMO because the data is also called email and password. "Prefilled" makes clear that it's just the initial value.

@distantnative
Copy link
Member

The data object is only internally and is user.email and user.password.

Alternatively, the prop would be value with an object as well to be more in line with our other components.

@lukasbestle
Copy link
Member Author

Fine for me. Went with a value object so we can also prefill the code if needed.

distantnative
distantnative previously approved these changes Mar 24, 2024
Copy link
Member

@distantnative distantnative left a comment

Choose a reason for hiding this comment

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

@lukasbestle looks good to me, added some inline docs for the props when we're already touching this. Also switched to inherit props to make them more consistent - we're trying to do it like that in more places recently.

@lukasbestle
Copy link
Member Author

Looks good, thanks for the changes.

@distantnative distantnative merged commit 10f1fd7 into develop-minor Mar 24, 2024
4 checks passed
@distantnative distantnative deleted the feature/prefilled-login-form branch March 24, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants