-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Tailwind #446
Tailwind #446
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 👏🏼
Left a few suggestions re translations
className='es-rounded-3! es-mx-auto! es-font-weight-400 es-color-cool-gray-500! es-nested-color-current!' | ||
isColumnLayout={true} | ||
> | ||
{sprintf(__('Form "%s" with type "%s" will be displayed here.', 'eightshift-forms'), formsFormPostIdRaw?.label, formsFormPostIdRaw?.metadata)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Translator info missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is JS file it is not req here
label={<span className='es-font-weight-400'>{__('Eightshift Forms', 'eightshift-forms')}</span>} | ||
className='es-rounded-3! es-mt-5! es-mx-auto! es-font-weight-400 es-color-cool-gray-500! es-nested-color-current!' | ||
> | ||
{sprintf(__('Form "%s" with type "%s" will be displayed here.', 'eightshift-forms'), item?.form?.label, item?.form?.metadata)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Translator info missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is JS file it is not req here
Helpers::selector($selectorClass, $selectorClass, $componentClass), | ||
Helpers::selector($additionalErrorClass, $additionalErrorClass), | ||
Helpers::selector($additionalClass, $additionalClass), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helpers::selector($additionalClass, $additionalClass), | |
$additionalClass, |
Please do a find-replace for this in the whole plugin. The classnames
handle will handle empty/null values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do this in the refactoring of the components with updates on the libs, I will skip this at this point.
key={index} | ||
icon={icons.form} | ||
label={<span className='es-font-weight-400'>{__('Eightshift Forms', 'eightshift-forms')}</span>} | ||
className='es-rounded-3! es-mt-5! es-mx-auto! es-font-weight-400 es-color-cool-gray-500! es-nested-color-current!' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be replaced with TW at some point, but let's leave them as-is for now 😄
Added
Fixed
Removed