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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Editable #890

Merged
merged 24 commits into from
May 13, 2024
Merged

Feature: Editable #890

merged 24 commits into from
May 13, 2024

Conversation

epr3
Copy link
Collaborator

@epr3 epr3 commented May 2, 2024

Implements #384.

This PR implements an Editable component with stories, tests and documentation.

The component anatomy is as following:

<EditableRoot>
  <EditableArea>
     <EditablePreview />
     <EditableInput />
  </EditableArea>
  <EditableEditTrigger />
  <EditableSubmitTrigger />
  <EditableCancelTrigger />
</EditableRoot>

I've skipped the properties related to styling like autoResize because I think that the styling is the responsibility of the user, this being a headless component library.

Any feedback is very appreciated. Thanks! 馃殌

@epr3 epr3 marked this pull request as draft May 2, 2024 16:45
@epr3 epr3 marked this pull request as ready for review May 9, 2024 16:04
@epr3 epr3 requested a review from zernonia May 9, 2024 16:04
@epr3 epr3 changed the title Draft: feat(Editable): implement Editable component feat(Editable): implement Editable component May 9, 2024
@epr3 epr3 changed the title feat(Editable): implement Editable component Feature: Editable May 9, 2024
@zernonia
Copy link
Collaborator

@epr3 without the maxWidth & autoResize prop, how can we achieve this with CSS only yaa? I had some difficulty replicate what Zagjs has 馃槄

@epr3
Copy link
Collaborator Author

epr3 commented May 10, 2024

@zernonia You're right, the autoResize prop is not very achievable using CSS since it relies on a visibility: hidden on the EditablePreview component to increase the size of the wrapping EditableArea thus expanding the available area for the EditableInput.

I guess the maxWidth property is not required since you can add a class on EditableArea to implement it and I think there is an issue on the Zagjs implementation because setting maxWidth doesn't clamp the field at all.

Relevant Stackblitz: https://stackblitz.com/edit/vitejs-vite-xoxacp?file=src%2FApp.vue&terminal=dev

I am going to add the autoResize property plus variants to Chromatic and some tests today.

EDIT: autoResize has been implemented.

@zernonia zernonia changed the base branch from main to 1.8 May 13, 2024 10:27
Copy link
Collaborator

@zernonia zernonia left a comment

Choose a reason for hiding this comment

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

Really nicely done @epr3 !! Thanks for the awesome job!

@zernonia zernonia merged commit 362930e into radix-vue:1.8 May 13, 2024
2 of 3 checks passed
@epr3 epr3 deleted the 384-feature-editable branch May 13, 2024 17:10
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.

None yet

2 participants