Skip to content

Commit 403dacb

Browse files
authored
Several concurrent bug fixes related to shipping of AddonHeader changes. (#13655)
* fixes: - remove number from users badge when no active users - install button does not shrink in loading state - add "Experimental" test on experimental badge - reformat grid names and spacing to make it more clear the structure of the layout - make layout more consistent with figma * static addon card * Fixes * Stop flipping badge direction and minor style tweak to badge padding, border radius * Add some tests. * Add fix for static themes on StaticAddonCard fixing alignment of rpeview image and right aligned badges. * AddonBadges aligned to start of flex container. * Add gap instead of margin to Addon-warnings content to prevent shadow margin when empty. * Use readable grid area names * Fix icon spacing on RTL using appropriate abstract specifiers. * Remove GetFirefoxCallout as it totally breaks the layout and is absolutely not worth it. * Add text wrapping to badge content (for suppppper small screens) * Prevent laoding button from collapsing to fixed width * Fix inner gap between theme preview image and addon info (title/description) * Revert "Remove GetFirefoxCallout as it totally breaks the layout and is absolutely not worth it." This reverts commit de54aa9.
1 parent e98f161 commit 403dacb

File tree

10 files changed

+303
-218
lines changed

10 files changed

+303
-218
lines changed

src/amo/components/AddonBadges/index.js

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { reviewListURL } from 'amo/reducers/reviews';
88
import { CLIENT_APP_FIREFOX } from 'amo/constants';
99
import translate from 'amo/i18n/translate';
1010
import { getPromotedCategory } from 'amo/utils/addons';
11-
import Badge, { BadgeIcon, BadgePill } from 'amo/components/Badge';
11+
import Badge from 'amo/components/Badge';
1212
import type { AppState } from 'amo/store';
1313
import type { AddonType } from 'amo/types/addons';
1414
import type { I18nType } from 'amo/types/i18n';
@@ -71,13 +71,7 @@ export class AddonBadgesBase extends React.Component<InternalProps> {
7171
type="experimental-badge"
7272
label={i18n.gettext('Experimental')}
7373
size="large"
74-
>
75-
{(props) => (
76-
<BadgePill {...props}>
77-
<BadgeIcon {...props} />
78-
</BadgePill>
79-
)}
80-
</Badge>
74+
/>
8175
);
8276
}
8377

@@ -149,13 +143,18 @@ export class AddonBadgesBase extends React.Component<InternalProps> {
149143

150144
const averageDailyUsers = addon.average_daily_users;
151145

152-
const userCount = i18n.formatNumber(averageDailyUsers);
153-
const userTitle =
146+
const userLabel =
154147
averageDailyUsers > 0
155-
? i18n.ngettext('User', 'Users', averageDailyUsers)
148+
? i18n.sprintf(
149+
i18n.ngettext(
150+
'%(count)s User',
151+
'%(count)s Users',
152+
averageDailyUsers,
153+
),
154+
{ count: i18n.formatNumber(averageDailyUsers) },
155+
)
156156
: i18n.gettext('No Users');
157157

158-
const userLabel = `${userCount} ${userTitle}`;
159158
return <Badge type="user-fill" label={userLabel} size="large" />;
160159
}
161160

src/amo/components/AddonBadges/styles.scss

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
display: flex;
55
flex-wrap: wrap;
66
gap: 6px;
7-
justify-items: start;
8-
align-items: center;
7+
align-items: flex-start;
8+
align-content: flex-start;
99

1010
.Addon-theme & {
1111
display: flex;

src/amo/components/Badge/styles.scss

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ $badge-size-small: 16px;
1111
gap: 6px;
1212
align-items: center;
1313
white-space: nowrap;
14-
padding: 4px 6px;
14+
padding: 3px 6px 3px 3px;
1515

1616
.Badge-link {
1717
color: inherit;
@@ -40,7 +40,7 @@ $badge-size-small: 16px;
4040

4141
&.Badge-border {
4242
border: 1px solid $color-light-gray-40;
43-
border-radius: 20px;
43+
border-radius: 22px;
4444
display: inline-flex;
4545
gap: 6px;
4646
box-sizing: border-box;
@@ -54,7 +54,6 @@ $badge-size-small: 16px;
5454

5555
@include respond-to(large) {
5656
display: flex;
57-
flex-direction: row-reverse;
5857

5958
.Addon-theme & {
6059
flex-direction: row;
@@ -66,6 +65,10 @@ $badge-size-small: 16px;
6665
}
6766
}
6867

68+
.Badge-content {
69+
text-wrap: wrap;
70+
}
71+
6972
.Badge-content--small {
7073
font-size: 12px;
7174
}

src/amo/pages/Addon/index.js

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -490,26 +490,28 @@ export class AddonBase extends React.Component {
490490

491491
<div>
492492
<Card className="Addon-header-info-card" photonStyle>
493-
<AddonInstallError error={this.props.installError} />
494-
495-
<AddonCompatibilityError addon={addon} />
496-
497-
{addon &&
498-
(addon.status !== STATUS_PUBLIC || addon.is_disabled) ? (
499-
<Notice type="error" className="Addon-non-public-notice">
500-
{i18n.gettext(
501-
'This is not a public listing. You are only seeing it because of elevated permissions.',
502-
)}
503-
</Notice>
504-
) : null}
505-
506-
{addon && <InstallWarning addon={addon} />}
507-
{addon ? (
508-
<WrongPlatformWarning
509-
addon={addon}
510-
className="Addon-WrongPlatformWarning"
511-
/>
512-
) : null}
493+
<div className="Addon-warnings">
494+
<AddonInstallError error={this.props.installError} />
495+
496+
<AddonCompatibilityError addon={addon} />
497+
498+
{addon &&
499+
(addon.status !== STATUS_PUBLIC || addon.is_disabled) ? (
500+
<Notice type="error" className="Addon-non-public-notice">
501+
{i18n.gettext(
502+
'This is not a public listing. You are only seeing it because of elevated permissions.',
503+
)}
504+
</Notice>
505+
) : null}
506+
507+
{addon && <InstallWarning addon={addon} />}
508+
{addon ? (
509+
<WrongPlatformWarning
510+
addon={addon}
511+
className="Addon-WrongPlatformWarning"
512+
/>
513+
) : null}
514+
</div>
513515

514516
<header className="Addon-header">
515517
{this.headerImage()}

src/amo/pages/Addon/styles.scss

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -35,69 +35,75 @@
3535
margin: 0;
3636
margin-bottom: 12px;
3737
}
38+
39+
.Card-contents {
40+
display: flex;
41+
flex-direction: column;
42+
gap: 12px;
43+
}
3844
}
3945

4046
.Addon-header {
4147
display: flex;
4248
flex-direction: column;
4349
row-gap: 14px;
50+
column-gap: 0;
4451

4552
@include respond-to(medium) {
4653
display: grid;
54+
55+
// stylelint-disable-next-line named-grid-areas-no-invalid
4756
grid-template:
48-
'preview preview' auto
49-
'icon icon' auto
50-
'info info' auto
51-
'badges badges' auto
52-
'. button' auto / 1fr 1fr;
57+
'theme theme theme' auto
58+
'icon info info' auto
59+
'badge badge badge' auto
60+
'. . install' auto / auto 1fr 1fr;
5361
}
5462

5563
@include respond-to(large) {
56-
column-gap: 14px;
5764
grid-template:
58-
'preview preview preview preview preview' auto
59-
'icon info info button button' auto
60-
'badges badges badges badges badges' auto / min-content 1fr 1fr 1fr auto;
65+
'theme theme theme theme theme' auto
66+
'icon info info info info' auto
67+
'badge badge badge . . ' auto
68+
'badge badge badge . install' auto / min-content 1fr 1fr 1fr 1fr;
6169
}
6270

6371
@include respond-to(extraLarge) {
6472
grid-template:
65-
'icon info button' auto
66-
'preview preview preview' auto
67-
'badges badges badges' auto / min-content 1fr auto;
73+
'theme theme theme theme theme . install' auto
74+
'icon info info info info . install' auto
75+
'badge badge badge badge badge badge badge' auto / min-content 1fr 1fr 1fr 1fr 1fr 1fr;
6876
}
6977

7078
@include respond-to(extraExtraLarge) {
79+
column-gap: 0;
80+
// stylelint-disable named-grid-areas-no-invalid
7181
grid-template:
72-
'preview preview preview preview . button' auto
73-
'icon info info info . button' auto
74-
'badges badges badges badges badges badges' auto / min-content 1fr 1fr 1fr auto;
82+
'theme theme theme theme install install' auto
83+
'icon info info info . install' auto
84+
'badge badge badge badge badge badge' auto / min-content 1fr 1fr 1fr 1fr 1fr;
85+
// stylelint-enable named-grid-areas-no-invalid
7586
}
7687

7788
& .Addon-theme-thumbnail {
78-
grid-area: preview;
89+
grid-area: theme;
7990
}
8091

8192
& .Addon-icon-wrapper {
8293
grid-area: icon;
94+
@include margin-end(16px);
8395
}
8496

8597
& .Addon-info {
8698
grid-area: info;
8799
}
88100

89101
& .AddonBadges {
90-
grid-area: badges;
102+
grid-area: badge;
91103
}
92104

93105
& .Addon-install {
94-
grid-area: button;
95-
@include respond-to(large) {
96-
min-width: 180px;
97-
}
98-
@include respond-to(extraLarge) {
99-
min-width: 200px;
100-
}
106+
grid-area: install;
101107
}
102108
}
103109

@@ -111,12 +117,6 @@
111117
width: 100%;
112118
}
113119

114-
.Addon .AMInstallButton-loading-button {
115-
@include respond-to(medium) {
116-
width: 48px;
117-
}
118-
}
119-
120120
// Details section with lots of grid stuff, on larger displays.
121121
@include respond-to(large) {
122122
.Addon-screenshots {

src/blog-utils/StaticAddonCard/index.js

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,16 @@ import { getAddonIconUrl } from 'amo/imageUtils';
77
import AddonBadges from 'amo/components/AddonBadges';
88
import AddonTitle from 'amo/components/AddonTitle';
99
import GetFirefoxButton from 'amo/components/GetFirefoxButton';
10-
import Rating from 'amo/components/Rating';
1110
import ThemeImage from 'amo/components/ThemeImage';
12-
import translate from 'amo/i18n/translate';
1311
import type { AddonType } from 'amo/types/addons';
14-
import type { I18nType } from 'amo/types/i18n';
1512

1613
import './styles.scss';
1714

1815
type Props = {|
1916
addon: AddonType,
2017
|};
2118

22-
type InternalProps = {|
23-
...Props,
24-
i18n: I18nType,
25-
|};
26-
27-
export const StaticAddonCardBase = ({
28-
addon,
29-
i18n,
30-
}: InternalProps): React.Node => {
19+
export const StaticAddonCardBase = ({ addon }: Props): React.Node => {
3120
if (!addon) {
3221
return null;
3322
}
@@ -65,14 +54,6 @@ export const StaticAddonCardBase = ({
6554
<p>{addon.summary}</p>
6655
</div>
6756

68-
<div className="StaticAddonCard-metadata">
69-
<Rating rating={addon.ratings.average} readOnly styleSize="small" />
70-
71-
<p className="StaticAddonCard-metadata-adu">
72-
Users: {i18n.formatNumber(addon.average_daily_users)}
73-
</p>
74-
</div>
75-
7657
<div className="StaticAddonCard-firefox-button">
7758
<GetFirefoxButton
7859
addon={addon}
@@ -91,7 +72,5 @@ export const StaticAddonCardBase = ({
9172
);
9273
};
9374

94-
const StaticAddonCard: React.ComponentType<Props> =
95-
translate()(StaticAddonCardBase);
96-
75+
const StaticAddonCard: React.ComponentType<Props> = StaticAddonCardBase;
9776
export default StaticAddonCard;

0 commit comments

Comments
 (0)