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

[MM-21749] [MM-22214] Add "Beta" and "Community" label #37

Merged
merged 5 commits into from
Feb 5, 2020

Conversation

hanzei
Copy link
Collaborator

@hanzei hanzei commented Jan 21, 2020

Summary

This PR allows adding a "Beta" and a "Community" label to plugin.

It currently looks like this: (JIRA is just an example)
Screenshot from 2020-02-01 09-29-10
Screenshot from 2020-02-01 09-29-14
Screenshot from 2020-02-01 09-29-18

Ticket Link

https://mattermost.atlassian.net/browse/MM-21749
https://mattermost.atlassian.net/browse/MM-22214

@hanzei hanzei added the 1: UX Review Requires review by a UX Designer label Jan 21, 2020
@hanzei hanzei requested a review from asaadmahmood January 21, 2020 11:34
@hanzei
Copy link
Collaborator Author

hanzei commented Jan 21, 2020

@asaadmahmood This is the label we talked about.

@asaadmahmood
Copy link

@hanzei I think we should just have a dimmer label, like the bot label for the beta tag. The blue labels are way too prominent.

@hanzei
Copy link
Collaborator Author

hanzei commented Jan 27, 2020

@asaadmahmood 👍 for a dimmer label. Do you have a suggestion?

@asaadmahmood
Copy link

We can just use what we have.
So just use the Badge component and if we can't use that, then just give it these clases:
Screenshot 2020-01-27 at 2 34 46 PM

However, since we need a fixed background and color, we can give it a background of:

background: rgba(61, 60, 64, 0.15)
color: #444;

Screenshot 2020-01-27 at 2 33 54 PM

@hanzei
Copy link
Collaborator Author

hanzei commented Jan 27, 2020

Screenshot from 2020-01-27 10-41-32
Screenshot from 2020-01-27 10-41-35

@asaadmahmood It looks very similar to the bot label with this setting. Would you be fine it I just re use the style of the bot label?

@asaadmahmood
Copy link

Yes, we should resuse the style of the bot label, but I gave the background and color because if we use the bot label directly without giving it an inline style, the theme may overwrite the background and the color, so try it with a theme and see if the bg and color are consistent.

Also, the local tag looks quite big compared to the bot label. So we can make them of similar size.

@hanzei
Copy link
Collaborator Author

hanzei commented Jan 27, 2020

I've applied the same background as the local label. This is how it looks like with a dark theme:
Screenshot from 2020-01-27 10-50-36
Screenshot from 2020-01-27 10-50-31

The background and color don't get overwritten by the theme.

@asaadmahmood
Copy link

@hanzei Can I have a full page screenshot?

@hanzei
Copy link
Collaborator Author

hanzei commented Jan 27, 2020

Screenshot from 2020-01-27 11-01-41

@asaadmahmood
Copy link

This looks weird.
I'll get a pull and look at it locally to fix the dark theme.

@hanzei hanzei force-pushed the MM-21749_beta-plugins branch from a2d1b98 to 37ca3d5 Compare January 31, 2020 18:50
@hanzei hanzei changed the title Add beta label [MM-21749][MM-22214] Add "Beta" and "Community" label Feb 1, 2020
@hanzei hanzei changed the title [MM-21749][MM-22214] Add "Beta" and "Community" label [MM-21749] [MM-22214] Add "Beta" and "Community" label Feb 1, 2020
@hanzei hanzei added 2: Dev Review Requires review by a core committer and removed 1: UX Review Requires review by a UX Designer labels Feb 4, 2020
internal/model/label.go Show resolved Hide resolved
internal/model/label.go Outdated Show resolved Hide resolved
cmd/generator/add.go Outdated Show resolved Hide resolved
cmd/generator/add.go Outdated Show resolved Hide resolved
@@ -81,12 +103,21 @@ var addCmd = &cobra.Command{
return errors.Wrap(err, "failed to download plugin signature")
}

labels := []model.Label{}
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
labels := []model.Label{}
var labels []model.Label

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think there is a noticeable nuisance here: var labels []model.Label will render to null in json, while labels := []model.Label{} renders to [] in json. I'm slightly favouring the latter as null might be confusing to read. 1/5

Copy link
Member

Choose a reason for hiding this comment

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

Ah, do we not omitempty for this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, we don't (https://github.com/mattermost/mattermost-marketplace/blob/master/internal/model/plugin.go#L17). We also don't omitempty for any other field. Hence, I did not add it for consistency.

cmd/generator/add.go Outdated Show resolved Hide resolved
internal/model/label.go Outdated Show resolved Hide resolved
@hanzei hanzei requested a review from lieut-data February 4, 2020 16:47
@hanzei hanzei requested a review from jfrerich February 4, 2020 18:07
@hanzei
Copy link
Collaborator Author

hanzei commented Feb 4, 2020

@jfrerich Would you mind taking a look at this PR given your involvement into plugin reviews?

@hanzei hanzei added the 1: PM Review Requires review by a product manager label Feb 4, 2020
@aaronrothschild aaronrothschild removed the 1: PM Review Requires review by a product manager label Feb 4, 2020
@jfrerich jfrerich added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Feb 5, 2020
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the review request too

@jfrerich jfrerich merged commit 66ba199 into master Feb 5, 2020
@jfrerich jfrerich deleted the MM-21749_beta-plugins branch February 5, 2020 18:37
hanzei added a commit that referenced this pull request Mar 24, 2020
* [MM-21749] [MM-22214] Add "Beta" and "Community" label (#37)

* Add beta and community label

* wording

* Feedback

* Make use of model.IsValid() (#44)

* Pin github.com/rakyll/statik version (#47)

* document automatic deployment (#46)

* document automatic deployment

* tweak merging language

Co-authored-by: Ben Schumacher <[email protected]>

* Add issue templates for community plugins (#45)

Add two issue templates: one for adding a new community plugin and one for updating an existing community plugin.

* [MM-23184] Omit empty labels list (#48)

Co-authored-by: Jesse Hallam <[email protected]>
lieut-data added a commit that referenced this pull request Jul 13, 2020
* [MM-21749] [MM-22214] Add "Beta" and "Community" label (#37)

* Add beta and community label

* wording

* Feedback

* Make use of model.IsValid() (#44)

* Pin github.com/rakyll/statik version (#47)

* document automatic deployment (#46)

* document automatic deployment

* tweak merging language

Co-authored-by: Ben Schumacher <[email protected]>

* Add issue templates for community plugins (#45)

Add two issue templates: one for adding a new community plugin and one for updating an existing community plugin.

* [MM-23184] Omit empty labels list (#48)

* Drop iconPaths workaround (#62)

* Use GITHUB_TOKEN env variable instead of having the user define it (#54)

* Proxy upstream support (#60)

* s/store/staticStore/

* prefer later plugins if version exactly matches

* introduce merged and proxy stores

* support --upstream

* generate on run-server

* update to go1.14

* tidy up interface/comments

* golangci-lint local-only complaints

* Update internal/store/proxy_test.go

* revert generate on run-server

* avoid merged store unless needed

Co-authored-by: Ben Schumacher <[email protected]>
Co-authored-by: mattermod <[email protected]>

* Ensure manually added plugins stay in the database (#68)

* Send Announcements for new plugins (#76)

* Limit enterprise plugins to E20 installations (#79)

* Limit enterprise plugins to E20 installations

* Apply suggestions from code review

Co-authored-by: Aaron Rothschild <[email protected]>

* Make getPlugins signature more clear

* Don't omit enterprise in json

* make generate

* Include workaround for https://mattermost.atlassian.net/browse/MM-26507

* Fix typo

* Changes as requested

* Drop else

* Move minVersionSupportingEnterpriseFlags into global scope

Co-authored-by: Aaron Rothschild <[email protected]>

* Add Enterprise label to MS Calendar

* make generate

Co-authored-by: Jesse Hallam <[email protected]>
Co-authored-by: mattermod <[email protected]>
Co-authored-by: Aaron Rothschild <[email protected]>
lieut-data added a commit that referenced this pull request Jul 13, 2020
* Merge master into production (#49)

* [MM-21749] [MM-22214] Add "Beta" and "Community" label (#37)

* Add beta and community label

* wording

* Feedback

* Make use of model.IsValid() (#44)

* Pin github.com/rakyll/statik version (#47)

* document automatic deployment (#46)

* document automatic deployment

* tweak merging language

Co-authored-by: Ben Schumacher <[email protected]>

* Add issue templates for community plugins (#45)

Add two issue templates: one for adding a new community plugin and one for updating an existing community plugin.

* [MM-23184] Omit empty labels list (#48)

Co-authored-by: Jesse Hallam <[email protected]>

* Add v0.14.0 of mattermost-plugin-github to the Marketplace (#51)

* Add v1.3.0 of mattermost-plugin-zoom to the Marketplace (#50)

* Add v1.2.0 of mattermost-plugin-custom-attributes to the Marketpl… (#53)

* Add mattermost plugin autolink v1.2.0 (#56)

* Add v1.2.0 of mattermost-plugin-autolink to the Marketplace

* Remove custom-attributes from this PR

* add the statik.go file after manually removed autolink from plugins.json and reran 'make generate'

Co-authored-by: Ben Schumacher <[email protected]>

* Add v1.3.1 of mattermost-plugin-zoom to the Marketplace (#59)

* Add mattermost plugin todo v0.2.0 (#57)

* Add v2.4.0 of mattermost-plugin-jira to the Marketplace (#66)

* Add v1.0.4 of mattermost-plugin-nps to the Marketplace (#63)

* Add v0.2.0 of mattermost-plugin-skype4business to the Marketplace (#70)

* Add v1.0.0 of mattermost-plugin-github to the Marketplace (#69)

* Add v1.2.0 of mattermost-plugin-confluence to the Marketplace (#73)

* Add mattermost plugin welcomebot v1.2.0 (#71)

* Add v1.0.0 of mattermost-plugin-mscalendar to the Marketplace (#78)

* Add v1.1.0 of mattermost-plugin-jenkins to the Marketplace (#80)

* Add v2.0.0 of mattermost-plugin-jitsi to the Marketplace (#82)

* Add Enterprise label to MS Calendar

* make generate

* Add v1.3.0 of mattermost-plugin-giphy-moussetc to the Marketplace (#85)

Co-authored-by: Jesse Hallam <[email protected]>
Co-authored-by: Jason Frerich <[email protected]>
lieut-data added a commit that referenced this pull request Jul 13, 2020
* [MM-21749] [MM-22214] Add "Beta" and "Community" label (#37)

* Add beta and community label

* wording

* Feedback

* Make use of model.IsValid() (#44)

* Pin github.com/rakyll/statik version (#47)

* document automatic deployment (#46)

* document automatic deployment

* tweak merging language

Co-authored-by: Ben Schumacher <[email protected]>

* Add issue templates for community plugins (#45)

Add two issue templates: one for adding a new community plugin and one for updating an existing community plugin.

* [MM-23184] Omit empty labels list (#48)

* Drop iconPaths workaround (#62)

* Use GITHUB_TOKEN env variable instead of having the user define it (#54)

* Proxy upstream support (#60)

* s/store/staticStore/

* prefer later plugins if version exactly matches

* introduce merged and proxy stores

* support --upstream

* generate on run-server

* update to go1.14

* tidy up interface/comments

* golangci-lint local-only complaints

* Update internal/store/proxy_test.go

* revert generate on run-server

* avoid merged store unless needed

Co-authored-by: Ben Schumacher <[email protected]>
Co-authored-by: mattermod <[email protected]>

* Ensure manually added plugins stay in the database (#68)

* Send Announcements for new plugins (#76)

* Limit enterprise plugins to E20 installations (#79)

* Limit enterprise plugins to E20 installations

* Apply suggestions from code review

Co-authored-by: Aaron Rothschild <[email protected]>

* Make getPlugins signature more clear

* Don't omit enterprise in json

* make generate

* Include workaround for https://mattermost.atlassian.net/browse/MM-26507

* Fix typo

* Changes as requested

* Drop else

* Move minVersionSupportingEnterpriseFlags into global scope

Co-authored-by: Aaron Rothschild <[email protected]>

* Merge production into master (#88)

* Merge master into production (#49)

* [MM-21749] [MM-22214] Add "Beta" and "Community" label (#37)

* Add beta and community label

* wording

* Feedback

* Make use of model.IsValid() (#44)

* Pin github.com/rakyll/statik version (#47)

* document automatic deployment (#46)

* document automatic deployment

* tweak merging language

Co-authored-by: Ben Schumacher <[email protected]>

* Add issue templates for community plugins (#45)

Add two issue templates: one for adding a new community plugin and one for updating an existing community plugin.

* [MM-23184] Omit empty labels list (#48)

Co-authored-by: Jesse Hallam <[email protected]>

* Add v0.14.0 of mattermost-plugin-github to the Marketplace (#51)

* Add v1.3.0 of mattermost-plugin-zoom to the Marketplace (#50)

* Add v1.2.0 of mattermost-plugin-custom-attributes to the Marketpl… (#53)

* Add mattermost plugin autolink v1.2.0 (#56)

* Add v1.2.0 of mattermost-plugin-autolink to the Marketplace

* Remove custom-attributes from this PR

* add the statik.go file after manually removed autolink from plugins.json and reran 'make generate'

Co-authored-by: Ben Schumacher <[email protected]>

* Add v1.3.1 of mattermost-plugin-zoom to the Marketplace (#59)

* Add mattermost plugin todo v0.2.0 (#57)

* Add v2.4.0 of mattermost-plugin-jira to the Marketplace (#66)

* Add v1.0.4 of mattermost-plugin-nps to the Marketplace (#63)

* Add v0.2.0 of mattermost-plugin-skype4business to the Marketplace (#70)

* Add v1.0.0 of mattermost-plugin-github to the Marketplace (#69)

* Add v1.2.0 of mattermost-plugin-confluence to the Marketplace (#73)

* Add mattermost plugin welcomebot v1.2.0 (#71)

* Add v1.0.0 of mattermost-plugin-mscalendar to the Marketplace (#78)

* Add v1.1.0 of mattermost-plugin-jenkins to the Marketplace (#80)

* Add v2.0.0 of mattermost-plugin-jitsi to the Marketplace (#82)

* Add Enterprise label to MS Calendar

* make generate

* Add v1.3.0 of mattermost-plugin-giphy-moussetc to the Marketplace (#85)

Co-authored-by: Jesse Hallam <[email protected]>
Co-authored-by: Jason Frerich <[email protected]>

* s/sls/serverless

Co-authored-by: Ben Schumacher <[email protected]>
Co-authored-by: mattermod <[email protected]>
Co-authored-by: Aaron Rothschild <[email protected]>
Co-authored-by: Jason Frerich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants