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

create input component #61

Merged
merged 25 commits into from
Aug 2, 2022
Merged

create input component #61

merged 25 commits into from
Aug 2, 2022

Conversation

Magnusrm
Copy link
Contributor

@Magnusrm Magnusrm commented Jul 6, 2022

Create input component using design system

Screen Recording 2022-07-07 at 12 20 08

image

Issue

fixes #47 Input component

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

@Magnusrm Magnusrm marked this pull request as ready for review July 7, 2022 09:31
@Magnusrm Magnusrm requested a review from haakemon July 7, 2022 10:35
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

Codecov Report

Merging #61 (566c21a) into main (79f330d) will increase coverage by 0.92%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   96.81%   97.73%   +0.92%     
==========================================
  Files          12       15       +3     
  Lines         157      221      +64     
  Branches       36       58      +22     
==========================================
+ Hits          152      216      +64     
  Misses          5        5              
Impacted Files Coverage Δ
src/components/TextField/Icon.tsx 100.00% <100.00%> (ø)
src/components/TextField/TextField.tsx 100.00% <100.00%> (ø)
src/components/TextField/utils.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79f330d...566c21a. Read the comment docs.

@haakemon haakemon requested a review from olemartinorg July 22, 2022 12:22
haakemon added 2 commits July 22, 2022 15:12
…nually expose each native input property

This has a downside where NumberFormat component does not have as broad type definition, so we cannot pass 'rest' props to that component
@haakemon
Copy link
Contributor

Design system v2 is setting

font-size: inherit;
line-height: inherit;

which computes to 16px for font-size and 24px for line-height. This causes the inputfields to be much bigger when introduced in app-frontend repo. We need to look into how to deal with this, as the inputfields should be 36px tall. Currently they will be 40px instead because of the inherited values.

Copy link
Contributor

@haakemon haakemon left a comment

Choose a reason for hiding this comment

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

As mentioned previously, we need to fix the height issues

Copy link
Contributor

@olemartinorg olemartinorg left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🎉 Of course, the mentioned size issue should be looked at.

…ature/input-component

# Conflicts:
#	src/components/index.ts
…side element. Subtract the border width from the padding to fix the issue
@haakemon
Copy link
Contributor

haakemon commented Aug 2, 2022

Height issue has been partially fixed. It looks good when the component is used in app-frontend, but it doesnt look correct when viewed in Storybook.

Created a separate task to follow up this issue #76

@haakemon haakemon merged commit 191d6a4 into main Aug 2, 2022
@haakemon haakemon deleted the feature/input-component branch August 2, 2022 08:14
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.

Input component
4 participants