Improve phone input UI #8266
Improve phone input UI #8266
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR improves the phone input UI by adjusting spacing and styling, particularly focusing on the country code picker and input field layout.
- Modified margin-left in
PhonesFieldInput.tsxto add 2 units of spacing between country code picker and input field - Adjusted padding in
PhoneCountryPickerDropdownButton.tsxfor better visual balance (1.25 left, 1 right) - Changed
StyledInputContainerpadding to 0 inDropdownMenuInput.tsxto fix spacing issues - Added border-right styling to country picker dropdown button for visual separation
- Noted limitations of react-phone-number-input library regarding country code color customization
3 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| padding: 0 ${({ theme }) => theme.spacing(2)} | ||
| ${({ theme }) => theme.spacing(1)}; | ||
| padding: 0 ${({ theme }) => theme.spacing(2)}; | ||
| ${({ theme }) => theme.spacing(1)}; |
There was a problem hiding this comment.
syntax: Missing template literal - this line won't apply the spacing correctly
|
Thanks @Hitarthsheth07 for your contribution! Country CodeLet's keep it to color:primary then PaddingCurrent
The right padding is too large. DesiredIt should be 4px so that the icon button's top, left, and bottom padding are equal.
Dropdown menuCurrent
Width is 50 DesiredWidth should be 44
|
…eth07/twenty into improve-phone-input-ui
|
Thanks for the feedback @Bonapara Here are the changes: Dropdown menuI've added the left padding 8px and right padding 4x. The overall width is still 44.8px because it also takes into account the border. While in figma we are using gap to show border, so the overall width of the left container remains same. https://www.loom.com/share/11ddb9ba2402479bbdb6b306e30215cc?sid=5b0da58a-42f3-4bf8-8fa8-21bf870c72e2 This could be solved by changing the bg color of the parent and children div and then adding gap to them, similar approach to what you're doing in figma. Right Add iconAs suggested padding on all the sides is even now. |
Hitarthsheth07
force-pushed
the
improve-phone-input-ui
branch
from
5eb21eb
to
e89c5fb
Compare
|
Hi @Hitarthsheth07, regarding the padding, I'm referring to the gap between the icon and input edge:
Also your PR seems to cause the picker to freeze when I click on the three dots menu: CleanShot.2024-11-04.at.10.38.52.mp4 |
Ahh got it! |
|
@Bonapara btw, can you check if there are any error in the browser console when you click on the three dots. I've only made changes to CSS styles and it shouldn't affect anything else. I do get error when clicked on three dots in my local setup and the app freezes without even changing any code. |
|
@Hitarthsheth07 The bug was actually caused by something else and has been fixed on the main branch. I updated your branch to the main, and the bug is gone. Let's forget about this issue and focus on fixing the icon padding! |
On it, will push the commit in an hour or two. |
…eth07/twenty into improve-phone-input-ui
| @@ -95,7 +103,9 @@ export const PhoneCountryPickerDropdownButton = ({ | |||
| <StyledDropdownButtonContainer isUnfolded={isDropdownOpen}> | |||
| <StyledIconContainer> | |||
| {selectedCountry ? <selectedCountry.Flag /> : <IconWorld />} | |||
| <IconChevronDown size={theme.icon.size.sm} /> | |||
| <StyledCheveronIconContainer> | |||
There was a problem hiding this comment.
Why did you add a styled container?
There was a problem hiding this comment.
The chevron down icon is not sized as per the figma file, the only way to change the size is by adding a container
|
It seems I have a 1px gap with the cell. Is this the case for you as well? It should be on top of the cell border. |
|
@Bonapara the gap between cell and dialog box is already there in this version. |
|
You're right! I think we will fix it in the release quick fixes, so no need thanks! |
Sounds good! Can this be merged and closed Thanks. |
|
@lucasbordeau if you can do the review - lgtm from a product perspective |
|
@lucasbordeau any updates on this PR? |
[FIX] #8251
Changes made as suggested by @Bonapara.
For the
The country code should be Tertiary instead of Primarytask,the library "react-phone-number-input" doesn't provide any out of the box functionality to style the country code.
If the feature needs to be implemented here are the possible solution/workarounds:
OR
OR
We'll have to get the length of the country code and then style the first X digits in the custom input field...
Let me know if someone has a better approach.