-
Notifications
You must be signed in to change notification settings - Fork 414
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
fix hover style for char count field #7620
fix hover style for char count field #7620
Conversation
thanks! I'll take a look. |
Hi @jessopb all good with this, do you need me to change something? |
I think this would be cleaner if it weren't class "button--file-action". It's not completely clear what file-action is, but this is a comment action or something. I think it deserves its own class. button--comment-icons, perhaps. |
Hello, @jessopb I have already sent the changes, is this ok? |
@ByronEricPerez I think it just needs a clean class that takes the place of button--file-actions because it's not a "file-action". |
Hello @jessopb , it is not entirely clear to me, would it be to rename the class? |
@@ -261,7 +261,7 @@ export class FormField extends React.PureComponent<Props> { | |||
{!noEmojis && openEmoteMenu && ( | |||
<Button | |||
type="alt" | |||
className="button--file-action" | |||
className="button--file-action button--comment-icons" |
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 should not need button--file-action
One class should be sufficient to specify what this should look like.
button--comment-icons should contain all that it needs.
This reverts commit 8a163e0.
Hi @jessopb , would that be ok? |
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.
Yeah, this is better. Just a couple more comments.
ui/scss/component/_button.scss
Outdated
height: initial; | ||
padding: 5px; | ||
|
||
&.button--comment-icons-active { |
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.
If you look in the code for button--file-action-active, you'll see that it's used like this:
<Button
title={__('I like this')}
authSrc="filereaction_like"
className={classnames('button--file-action button-like', {
'button--file-action-active': myReaction === REACTION_TYPES.LIKE,
})}
It does not seem that button--comment-icons-active
is used anywhere - I'm not sure it's necessary.
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 looks good!
a1fdbd4
to
d7fecaf
Compare
Fixes
Issue Number: #7564
What is the current behavior?
What is the new behavior?
Other information
PR Checklist
Toggle...
What kind of change does this PR introduce?
Please check all that apply to this PR using "x":