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.

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: also use custom private boost icon for detailed status #14471

Merged

Conversation

OmmyZhang
Copy link
Contributor

#14380
If the custom icon for private boosts is accepted, it should be used everywhere.

By the way, this change can fix #14455

@@ -20,6 +20,7 @@ export default class IconButton extends React.PureComponent {
style: PropTypes.object,
activeStyle: PropTypes.object,
disabled: PropTypes.bool,
privat: PropTypes.bool,
Copy link
Contributor

@brawaru brawaru Jul 29, 2020

Choose a reason for hiding this comment

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

You probably meant private. And you really shouldn't pass this to shared IconButton component, there's a class name for that, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, private is a reserved word in JS.

It's similar to passing disabled as a class name. I believe this make the code structure more clear.

@brawaru
Copy link
Contributor

brawaru commented Jul 29, 2020

How does that affect #14455? Also, ThibG has already created a PR to fix it — #14456.

@OmmyZhang
Copy link
Contributor Author

Because now custom icon is only used for button.icon-button.privat , and will not affect button.icon-button.disabled.

I have seen ThibG's PR, but my PR is mainly for using custom private boost icon everywhere.

@OmmyZhang
Copy link
Contributor Author

diff
master

fixed

@ClearlyClaire ClearlyClaire self-requested a review August 3, 2020 12:40
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Sorry for the late review! I concur with @Sasha-Sorokin, privat doesn't sound right. Also, I think it is far too specific to the boost icon to be part of IconButton's attributes, and should use className instead.

@OmmyZhang
Copy link
Contributor Author

@Sasha-Sorokin @ThibG
Thanks for your review! Is it a little better now?

@Gargron Gargron merged commit a3ec9af into mastodon:master Aug 24, 2020
@OmmyZhang OmmyZhang deleted the fix-private-boost-icon-everywhere branch August 24, 2020 12:15
thenameisnigel-old pushed a commit to ChatterlyOSE/Chatterly that referenced this pull request Sep 6, 2020
…#14471)

* use custom private boost icon for detail status

* only use className
thenameisnigel-old pushed a commit to ChatterlyOSE/Chatterly that referenced this pull request Sep 7, 2020
…#14471)

* use custom private boost icon for detail status

* only use className
Gargron pushed a commit that referenced this pull request Oct 19, 2020
* use custom private boost icon for detail status

* only use className
shouo1987 pushed a commit to CrossGate-Pawoo/mastodon that referenced this pull request Dec 7, 2022
…#14471)

* use custom private boost icon for detail status

* only use className
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retoot icon with lock shows when hovering on "no retoot" icon of other's followers-only toots.
4 participants