Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

style: improve question submission form UI #164

Merged
merged 6 commits into from
Sep 19, 2022

Conversation

iShibi
Copy link
Contributor

@iShibi iShibi commented Sep 7, 2022

The receny field will eventually get converted to a Date in the database. It's better to ask the specific date from the creator itself.

@vercel
Copy link

vercel bot commented Sep 7, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
defaang ✅ Ready (Inspect) Visit Preview Sep 18, 2022 at 7:05AM (UTC)

@ykdojo
Copy link
Collaborator

ykdojo commented Sep 7, 2022

I think it's better to keep the current way of asking it.

We shouldn't ask it in such a specific way that the person will be identifiable from the date. What do you think?

@subhoghoshX
Copy link
Collaborator

I agree with @ykdojo. And I don't think anyone would remember the exact date and might eventually be forced to just enter a random date or not change the date at all.

@iShibi
Copy link
Contributor Author

iShibi commented Sep 7, 2022

That's a valid point. I did overlook the impact it will have on a creator's privacy.

What we can do is remove the recency/asked_date altogether and let the reader measure the freshness of a question from its creation date.

This will save us from asking a specific question as well as from storing a vague field in the database.

@ykdojo
Copy link
Collaborator

ykdojo commented Sep 7, 2022

I think it is valuable to have this piece of info even if it's not exact, though?

@iShibi
Copy link
Contributor Author

iShibi commented Sep 8, 2022

It is valuable. My concern is more about the fact that we will have to generate an exact date using recency to store it in the DB, which is a little hacky. I suggest we ask for mm/yyyy. A month is not very specific and even after that if a user wants to be safe, they can use stay_anonymous option. What do you think?

@ykdojo
Copy link
Collaborator

ykdojo commented Sep 8, 2022

Asking for mm/yyyy might work. @subhoghoshX what do you think?

@subhoghoshX
Copy link
Collaborator

Yes, asking for mm/yyyy sounds good to me. It's more vague than say, within the past week but more exact compared to 3-6 months or 7-12 months.

You might have to implement a custom date picker. Let me know if you need any help with that.

@iShibi
Copy link
Contributor Author

iShibi commented Sep 10, 2022

The <input type='month'> isn't widely supported yet so I went with a combination of <select> and <input type='tel'> for month and year respectively. I also added some validation rules and error feedback.

Broke asked_date into asked_month (text) and asked_year (smallint). This will help in querying questions asked in a particular month or year independently.

@ykdojo @subhoghoshX

@ykdojo
Copy link
Collaborator

ykdojo commented Sep 11, 2022

@iShibi thank you.

One quick thing - I think it's important that the current month is selected by default so we don't add unnecessary friction to the whole process.

@subhoghoshX
Copy link
Collaborator

Also maybe the current year as well. I think it can also be a select input listing only last 5-10 years.

I'm still not sure about having two separate fields. Having one date field would also allow querying a particular month or year independently as well. Doesn't matter too much I guess.

Also, I think it might be better if the month is also an integer as a value from 1-12. Thanks.

@iShibi
Copy link
Contributor Author

iShibi commented Sep 11, 2022

Changes:

  1. Made asked_month of type smallint and number in SQL and TS respectively.
  2. Added a feature to pre-populate the month and year fields to the current month and year.

I kept the year field as <input type='tel'> because, unlike months, years are not limited. If we choose to make it <select>, the options will change over time and that's a recipe for bugs and forced limitation.

@subhoghoshX
Copy link
Collaborator

What I meant for the year is to generate it through JS. I meant something like (new Date()).getFullYear(). This will always return the present year, 2022 for now. And if the oldest year we allow supposing is 2015, we can generate the select options from 2015-2022 (for now). And as we're using Next.js it'll statically generate it, so no extra JavaScript will be there after build.

I'm not too sure about it. Open to discussion.

@iShibi
Copy link
Contributor Author

iShibi commented Sep 11, 2022

@subhoghoshX It's not about JS, in fact, clients will have to create year options during runtime otherwise the options will remain as it is forever if we never run the build again. My concerns are:

  1. We are forcing a limit on users for no reason whatsoever. The user should be able to submit a question that was asked to them no matter how long ago.
  2. Using select for months makes sense as it has a limited number of options that will remain constant forever but for years it does not seem the right choice.

@subhoghoshX
Copy link
Collaborator

Makes sense. I just wanted to limit what a user can enter even though we'd check it again through js or on the backend. It sucks that input type="number" doesn't work with maxlength property. I think we can at least apply maxlength="4" to the year field to limit it to 4 digits only.

I noticed you also made the inputs full width. Applying a little padding like p-12 on the container might make it look better.

supabase.sql Outdated Show resolved Hide resolved
@iShibi
Copy link
Contributor Author

iShibi commented Sep 11, 2022

I think we can at least apply maxlength="4" to the year field to limit it to 4 digits only.

@subhoghoshX This validation will make sure that the user can only submit a 4-digit year:
https://github.com/ykdojo/defaang/blob/ebbf87057f181cb16d5ebcf08cdf5ed4e04d0c37/src/components/QuestionSubmissionForm.tsx#L126

@subhoghoshX
Copy link
Collaborator

I think we can at least apply maxlength="4" to the year field to limit it to 4 digits only.

@subhoghoshX This validation will make sure that the user can only submit a 4-digit year:

https://github.com/ykdojo/defaang/blob/ebbf87057f181cb16d5ebcf08cdf5ed4e04d0c37/src/components/QuestionSubmissionForm.tsx#L126

Yeah I know we're doing it through js. I just wanted a visual indicator there that's all.

@iShibi
Copy link
Contributor Author

iShibi commented Sep 11, 2022

I just wanted a visual indicator there that's all

@subhoghoshX It does that too:

image

See:
https://github.com/ykdojo/defaang/blob/ebbf87057f181cb16d5ebcf08cdf5ed4e04d0c37/src/components/QuestionSubmissionForm.tsx#L141

@subhoghoshX
Copy link
Collaborator

@subhoghoshX It does that too:

Haha ok. No issues.

supabase.sql Outdated Show resolved Hide resolved
@ykdojo
Copy link
Collaborator

ykdojo commented Sep 17, 2022

Can we just keep the original method I came up with (asking about the time passed since the interview rather than the month)?

It'd be a more accurate approximation of the actual date with that method.

Related: #164 (comment)

@iShibi iShibi changed the title refactor: use date input type refactor: improve question submission form UI Sep 18, 2022
@ykdojo
Copy link
Collaborator

ykdojo commented Sep 19, 2022

It looks good but the UI changes right? So we should probably use the feat label, rather than refactor

@iShibi iShibi added style New CSS related changes or request and removed refactor labels Sep 19, 2022
@iShibi iShibi changed the title refactor: improve question submission form UI style: improve question submission form UI Sep 19, 2022
@iShibi iShibi merged commit bd83c19 into csdojo-defaang:main Sep 19, 2022
@Gulshanaggarwal
Copy link
Contributor

Gulshanaggarwal commented Sep 19, 2022

@ykdojo I think we must give option of company tags(initially for only a few), otherwise users will write companies names in different ways. for example - someone may write facebook or facebok. So it will make the Users experience inconsistent because they seeing same company name in different ways. And for reset of the companies they can enter manually if they don't find. What do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
style New CSS related changes or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants