Conversation
adrianthedev
left a comment
adrianthedev
left a comment
There was a problem hiding this comment.
- the text input size doesn't match figma. can you pease cehck the others too? (don't set the height. see what's wrong with the sizes (padding, line height, font-size))
- let's try to add the spinner element as an after element.
| attr_reader :loading | ||
|
|
||
| def loading? | ||
| !!Avo::ExecutionContext.new(target: loading, record: record, view: view, resource: resource).handle |
There was a problem hiding this comment.
Why the !!?
| !!Avo::ExecutionContext.new(target: loading, record: record, view: view, resource: resource).handle | |
| !!Avo::ExecutionContext.new(target: loading, record: record, view: view, resource: resource).handle |
| module IsLoading | ||
| extend ActiveSupport::Concern | ||
|
|
||
| attr_reader :loading |
There was a problem hiding this comment.
do we ever add this loading attribute? I don't recall having that.
Ideally, we should be triggering loading with just a class or HTML attribute, not by reloading the field from the server.
| <%= content %> | ||
| <div class="field-wrapper__input"> | ||
| <%= content %> | ||
| <%= tag.span class: "input__spinner", "aria-hidden": "true" if @field.loading? %> |
There was a problem hiding this comment.
Can we add the spinner as an after element and not have to depend on adding an extra element on the page?
…avo into avo-976-adjust-text-input-design
…avo into avo-976-adjust-text-input-design
adrianthedev
left a comment
adrianthedev
left a comment
There was a problem hiding this comment.
Let's have one input.css if we can. It's easier to co-locate these and figure out patterns now and in the future.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: |
There was a problem hiding this comment.
What is the reason for this?
| /* Input Component - Base */ | ||
| /* BEM: .input-field (block), .input-field__* (element), .input--* (modifier) */ | ||
|
|
||
| /* Replaced input heights (range, color): content + padding + border */ |
There was a problem hiding this comment.
| /* Input Component - Base */ | |
| /* BEM: .input-field (block), .input-field__* (element), .input--* (modifier) */ | |
| /* Replaced input heights (range, color): content + padding + border */ |
It doesn't seem that these help in any way.
| --input-height-sm: calc(1rem + 0.5rem + 2px); | ||
| --input-height-md: calc(1.25rem + 0.75rem + 2px); | ||
| --input-height-lg: calc(1.25rem + 1.25rem + 2px); |
There was a problem hiding this comment.
Can we figure these out from actual variables?
I will always wonder where does the 1rem come from. or the 0.5rem or the 2px.
Are these spacings? do they have variables in Tailwind? Can we leave comments on what they are?
| Color Input | ||
| ========================================================================== */ | ||
|
|
||
| input[type="color"] { |
There was a problem hiding this comment.
this is still an input. let's move it into input.css
| .input--size-sm ~ .input-field__password-toggle svg { | ||
| @apply size-3.5; | ||
| } | ||
|
|
||
| /* Medium size */ | ||
| .input-field ~ .input-field__password-toggle svg, | ||
| input ~ .input-field__password-toggle svg, | ||
| .input--size-md ~ .input-field__password-toggle svg { | ||
| @apply size-4; | ||
| } | ||
|
|
||
| /* Large size */ | ||
| .input--size-lg ~ .input-field__password-toggle svg { | ||
| @apply size-4.5; | ||
| } |
There was a problem hiding this comment.
can we have one CSS rule to dictate the size of the after element? I mean for password icon, calendar icon, loadign icon, etc.?
Also maybe make it a variable and let CSS apply it?
2 participants
Description
polish design specifications for text inputs based on figma https://www.figma.com/design/Enmp0p0u8wFG0SuysUDszX/Avo-Design-System?node-id=2934-86442&m=dev
Checklist:
Screenshots & recording