add metric if responsiveMetric is undefined #6490
Conversation
| parseMetricToNum(responsiveMetric || metric) / 2 - | ||
| parseMetricToNum(responsiveBorderMetric) / 2 | ||
| }px`; |
There was a problem hiding this comment.
Unfortunately this will break spacing if any non-px metric is passed, as 'px' is forcefully appended to the parsed number. But this is a more basic flaw in the border between implementation. I wonder if it makes sense to address it separately?
There was a problem hiding this comment.
@shb Im not sure what you mean by passing a non-px value? Are you referring to the t-shirt sizing we have like small medium? Can you give me an example case where it isn’t working so I can see what you are trying to pass through?
There was a problem hiding this comment.
@shb I see the issue sorry it didn't click earlier so if someone passes 5em the borderOffSet will not be correct because it is assuming px values. Alot of the math would also not be correct considering the borderMetric used to calculate the borderOffSet is in px Im thinking this may take some time in converting some of the values. Im inclined to open a separate issue to look into all of the values that would need to change and how we want to address this.
@jcfilben @ericsoderberghp
There was a problem hiding this comment.
Yeah I think we can handle as a separate issue
Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com>
Co-authored-by: Eric Soderberg <eric.soderberg@hpe.com>
|
linking new issue for non pixel values being passed to |
What does this PR do?
fixes bug when using
border=betweenandgapwith a pixel valueWhere should the reviewer start?
styledBox.js
What testing has been done on this PR?
wrote a test and storybook
How should this be manually tested?
storybook
Do Jest tests follow these best practices?
screenis used for querying.userEventis used in place offireEvent.asFragment()is used for snapshot testing.Any background context you want to provide?
In the If statement for
border="between"it usesresponsive metric to defineresponsiveBorderMetric` which is undefined when a user passes a pixel value.What are the relevant issues?
closes #6485
Screenshots (if appropriate)
no
Do the grommet docs need to be updated?
no
Should this PR be mentioned in the release notes?
no
Is this change backwards compatible or is it a breaking change?
backwards compatible