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.

Sign up for GitHub

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

Jump to bottom

Hotfix wrong override when value is not defined in breakpoint #6969

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Hotfix wrong override when value is not defined in breakpoint #6969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d6080ac
Hotfix wrong override when value is not defined in breakpoint
AntoineDuComptoirDesPharmacies Oct 12, 2023
8ac167c
Run yarn jest -u as requested by @britt6612.
AntoineDuComptoirDesPharmacies Oct 12, 2023
2c249a2
Update yarn lock and update yarn + max package size
AntoineDuComptoirDesPharmacies Oct 13, 2023
5eba190
Merge remote-tracking branch 'origin/master' into fix-breakpoint-defa…
AntoineDuComptoirDesPharmacies Oct 20, 2023
a402208
Fix tests
AntoineDuComptoirDesPharmacies Oct 20, 2023
c3d6a50
Merge remote-tracking branch 'origin/master' into fix-breakpoint-defa…
AntoineDuComptoirDesPharmacies Oct 24, 2023
ab629cc
Revert "Fix tests"This reverts commit a4022084e1adcb8aa58a9f5657f25f6…
AntoineDuComptoirDesPharmacies Oct 24, 2023
62a2c47
Put again 162 kB as it is modified in master
AntoineDuComptoirDesPharmacies Oct 24, 2023
02b0982
Remove useless style in tests
AntoineDuComptoirDesPharmacies Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/js/components/Tip/__tests__/__snapshots__/Tip-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,12 +579,6 @@ exports[`Tip themed 1`] = `
animation-delay: 0.01s;
}

@media only screen and (max-width:768px) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm trying to understand why this margin goes away with these changes. Can you explain?

.c0 {
margin: 21px;
}
}

@media only screen and (max-width:768px) {
.c2 {
margin: 3px;
Expand Down
12 changes: 7 additions & 5 deletions src/js/utils/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,19 @@ export const edgeStyle = (
responsiveBreakpoint,
theme,
) => {
const breakpoint =
responsiveBreakpoint && theme.global.breakpoints[responsiveBreakpoint];
const breakpoint = getBreakpointStyle(theme, responsiveBreakpoint);
const metric = theme.global.edgeSize[data] || data;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Behavior looks great. Two small naming suggestions would be to align the variable names with the concept of CSS value. Metric was a little bit confusing at first:

  • metric --> value
  • responsiveMetric --> responsiveValue

soft opinion.

const responsiveMetric = responsive && breakpoint &&
breakpoint.edgeSize[data];

if (typeof data === 'string') {
return css`
${kind}: ${theme.global.edgeSize[data] || data};
${responsive && breakpoint
${kind}: ${metric};
${responsiveMetric
? breakpointStyle(
breakpoint,
`
${kind}: ${breakpoint.edgeSize[data] || data};
${kind}: ${responsiveMetric};
`,
)
: ''};
Expand Down