added sharp styles #49
Conversation
| render() { | ||
| const falDisabled = !this.pro(); | ||
| const faslDisabled = !this.isSharpLightAvailable(); | ||
| const fassDisabled = !this.isSharpSolidAvailable(); | ||
| const fasrDisabled = !this.isSharpRegularAvailable(); | ||
| const fatDisabled = !(this.isV6() && this.pro()); | ||
| const fadDisabled = !this.isDuotoneAvailable(); | ||
| const fakDisabled = !this.mayHaveIconUploads(); |
There was a problem hiding this comment.
@mlwilkerson @frrrances Checked to make sure that the "pro" styles are disabled if no kit token is supplied
Screen.Recording.2023-03-27.at.5.14.29.PM.mov |
There was a problem hiding this comment.
oh... it feels good to get this updated. nice.
Made some comments requesting changes to minimum version requirements and semver satisfaction.
I think it might be good to add tests asserting the values produced by those computed properties: isSharpSolidAvailable, isSharpRegularAvailable, and isSharpLightAvailable. Seems like these are all too easy to mistake, so test coverage seems useful here. Or if we do already have test coverage there, then test fixes to ensure that we don't regress the related min version requirement bugs again, once they're fixed here.
packages/fa-icon-chooser/src/components/fa-icon-chooser/fa-icon-chooser.tsx
Outdated
Show resolved
Hide resolved
packages/fa-icon-chooser/src/components/fa-icon-chooser/fa-icon-chooser.tsx
Outdated
Show resolved
Hide resolved
packages/fa-icon-chooser/src/components/fa-icon-chooser/fa-icon-chooser.tsx
Outdated
Show resolved
Hide resolved
packages/fa-icon-chooser/src/components/fa-icon-chooser/fa-icon-chooser.e2e.ts
Show resolved
Hide resolved
readme.md
Outdated
| @@ -26,7 +26,7 @@ When used with a kit, everything is available, according to that kit's configura | |||
| - Font Awesome Pro | |||
| - Font Awesome 6 | |||
|
|
|||
| Including the Sharp Solid style, when using a Pro kit with Font Awesome 6.2.0 or newer! | |||
| Including the Sharp Regular & Light styles, when using a Pro kit with Font Awesome 6.4.0 or newer! | |||
There was a problem hiding this comment.
again FA 6.3.0 is the min version for Sharp Regular, while 6.4.0 is the min version for Sharp Light. This sentence doesn't distinguish them, so it sounds like they both first became available in 6.4.0.
There was a problem hiding this comment.
@mlwilkerson would it make sense just to remove that sentence?
There was a problem hiding this comment.
Actually, yeah. Removing it, it's not necessary now
There was a problem hiding this comment.
@kelseythejackson Nice work! This is super helpful. I marked one spot where there were console.log outputs to double-check but otherwise the code looks good.
But since these buttons add a lot more content width to the style select buttons area, we need to update the layout to manage them better. It looks like you have your browser full width, when this component usually renders at about the tablet width when in WordPress.
Do you want to take a pass as that work or would you like me to give you a mock to aim for?
| @@ -449,7 +456,18 @@ export class FaIconChooser { | |||
| } | |||
|
|
|||
| isSharpSolidAvailable() { | |||
| return this.pro() && !!this.resolvedVersion().match('(6.2[0-9]*.)'); | |||
| console.log(semver.satisfies(this.resolvedVersion(), '>=6.2.0')); | |||
There was a problem hiding this comment.
Should these console.log lines (here and below) stay in the finished code?
| @@ -26,7 +26,6 @@ When used with a kit, everything is available, according to that kit's configura | |||
| - Font Awesome Pro | |||
| - Font Awesome 6 | |||
|
|
|||
There was a problem hiding this comment.
Looks like there are a couple of extra returns in here (not from you). Mind cleaning those up while you're in here?
Description
This adds the Regular & Light styles for the Sharp family, as well as enables the Sharp Solid style. Two "bugs" are addressed in this PR:
Sharp Solid availability bug#47
Add Sharp Regular#48
Review
@mlwilkerson Could you take a look to make sure all spots that needed to be updated were hit?
@frrrances You were the designer on this, so I could use your eyes if anything was missed