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

[feature request] Improve CreateComment and CreateReply components #3656

Open
dalibormrska opened this issue Jun 12, 2024 · 7 comments
Open

Comments

@dalibormrska
Copy link
Collaborator

This ticket is part of series of 7 tickets concluding a design task focusing on improving design and usability of comments feature. Here you can find the Figma file for more context.

7/7 - Improve CreateComment and CreateReply components


Is your feature request related to a problem? Please describe.
The component is currently taking a lot of vertical space, since the button is below. The button is also too big visually on mobile. The textarea is not increasing the height automatically, so the user has to drag the bottom-right corner to re-size it. Additionally, its default height is too big for the text size and is making the text not centered.

image

Describe the solution you'd like
Figma link to the file where you can see the measurements. Click on the element and hold option mac key to see the measurements.

image

Ticket scope to include (notes from Ben):

  1. Update button component to choose to hide text on mobile viewpoint.
  2. Changing our textarea component (and implement for all instances) to include the character limit count.
  • Questions - title, description
  • how-to - title - Short description
  • how-to steps - title, description
  • research - title, description
  • research update - title, description
  • settings - Tell us a bit about yourself, Short description of your pin
  • Error handling
  • Rules around colours to use? e.g. red for under/near limit?

Describe alternatives you've considered
I think there are simpler alternatives, just changing the flex-direction so that the textarea and the button are next to each other, but I think this component deserves some love and nice styling. <3

Additional context
See the milestone

@elideg
Copy link

elideg commented Jun 20, 2024

@dalibormrska I can take this ticket on, do you mind assigning it to me?

@tomgiagtz
Copy link

Hey there! Any chance I could have this assigned to me? I should be able to get this done next week :)

@benfurber
Copy link
Member

@tomgiagtz Welcome! Good luck. Please shout if you have any questions/issues.

@tomgiagtz
Copy link

tomgiagtz commented Sep 4, 2024

Okay I made some progress yesterday and today, here it is so far!

image image image

I'm waiting to get the character count placed just right because...

I have 2 sticking points right now:

1- Auto Sizing Textarea / New Component

Auto Sizing the text area is fairly difficult it seems, especially with the intended functionality of expanding to a max size limit.

I found some discussion online, most people recommended using a package 👎 , but I found this method using the useEffect hook. I figure this is the best way to go as it will be easier to tie into theming but I wanted to bring it up incase I missed something pre-existing in the project.

Based on the widespread use of <Textarea/>, I'm thinking this deserves it's own component, let me know what you think.

2- Mobile Detection

I haven't looked through much of the existing codebase yet, but in order to implement the Mobile v2 design, I believe I will need some sort of isMobile boolean to be able to conditionally render the image, and remove the arrow and profile picture.

I see the profile image is already disappearing on smaller screen sizes, but based on the code in MemberBadge.tsx, I'm not sure that's intentional? Best guess is there isn't a low detail version of the default prof picture... This is what I'm seeing on smaller mobile sizes.
image

Is there some existing example of what I should do for this? Do I need to figure out how to get a media query to cover this functionality? Let me know!

Thanks, Tom

@benfurber
Copy link
Member

benfurber commented Sep 5, 2024

Awesome, thanks @tomgiagtz. Likes like you're making great progress.

On one, sounds like some great investigation. The suggestion of useEffect is a good one and definiting of moving as much as you can to do the component library (packages/components).

For the mobile stuff, I try to use the responsive element of sx to do that whereever possible. Basically whenever you see an array for a style element that's doing the responsive hard work. e.g. when you see something like:

sx: {
  padding: [1, 2, 3, 4],
}

That's setting a different padding across our four viewports.

I try to default to using that, even when there's enough complexity to justify doing entirely different mobile/desktop implementations like I did recently here.

Does all that make sense?

@benfurber
Copy link
Member

Hey @tomgiagtz, how are you getting on?

@benfurber
Copy link
Member

Hey @tomgiagtz, as we haven't heard back from you for a while I'm going to unassign you from the ticket. Please shout if you've able to help out again in the future. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New / Under Discussion
Development

Successfully merging a pull request may close this issue.

4 participants