Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Mm 29656 #7820

Merged
merged 8 commits into from
Apr 14, 2021
Merged

Mm 29656 #7820

merged 8 commits into from
Apr 14, 2021

Conversation

michelengelen
Copy link
Contributor

Summary

This PR is a replacement for #6942. Creating a new PR since the old one came from a fork and is stale. The original PR willbe closed after the creation of this. Dicsussions and comments will not be ported over, so please reference comments from the original PR if needed.

Ticket Link

Fixes MM-29656

Related Pull Requests

Original PR: #6942

@michelengelen michelengelen added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud 1: UX Review Requires review by a UX Designer labels Apr 6, 2021
@michelengelen michelengelen self-assigned this Apr 6, 2021
@michelengelen
Copy link
Contributor Author

/update-branch

@mattermod
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

# Conflicts:
#	components/product_notices_modal/__snapshots__/product_notices.test.tsx.snap
#	components/product_notices_modal/product_notices.test.tsx
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 6, 2021
@matthewbirtch matthewbirtch added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 6, 2021
@matthewbirtch
Copy link
Contributor

@michelengelen how do I test this to get the in-product notices to appear so I can review?

@michelengelen
Copy link
Contributor Author

Maybe @hmhealey can help. I would like to avoid hard-coding something that needs to be removed afterwards!

@michelengelen
Copy link
Contributor Author

Apparently it is not possible, so I will add a hard-coded part for now which I will remove before merging.

@esethna
Copy link
Contributor

esethna commented Apr 6, 2021

Thanks for the PR!

@michelengelen @matthewbirtch maybe a screenshot would be enough for UX review in this case?

@esethna esethna removed their request for review April 6, 2021 15:16
@matthewbirtch
Copy link
Contributor

Thanks for the PR!

@michelengelen @matthewbirtch maybe a screenshot would be enough for UX review in this case?

yep, that's fine with me if you want to provide screenshots @michelengelen

@michelengelen
Copy link
Contributor Author

@matthewbirtch here are some screenshots:

Primary Button

normal hover active focus
Screenshot 2021-04-06 at 17 50 02 Screenshot 2021-04-06 at 17 50 14 Screenshot 2021-04-06 at 17 50 25 Screenshot 2021-04-06 at 17 50 36

Secondary Button

normal hover active focus
Screenshot 2021-04-06 at 17 43 45 Screenshot 2021-04-06 at 17 44 35 Screenshot 2021-04-06 at 17 44 41 Screenshot 2021-04-06 at 17 47 12

Tertiary Button

normal hover active focus
Screenshot 2021-04-06 at 17 51 18 Screenshot 2021-04-06 at 17 51 25 Screenshot 2021-04-06 at 17 51 31 Screenshot 2021-04-06 at 17 51 53

@michelengelen michelengelen removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 6, 2021
@mm-cloud-bot
Copy link

Test server destroyed

Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Looks great based on the screenshots @michelengelen. Nice work. thanks!

@mm-cloud-bot
Copy link

Test server creation failed. See the logs for more information.

@esethna esethna removed the 1: UX Review Requires review by a UX Designer label Apr 6, 2021
Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thanks @michelengelen
Approving based on attached screenshots. QA will test after merge.

@jgilliam17 jgilliam17 added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Apr 7, 2021
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Looks good, is there anywhere else we need to add these styles? I know the GenericModal__button class is used in a few places.

@calebroseland
Copy link
Member

/update-branch

@calebroseland calebroseland added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter QA Review Done labels Apr 14, 2021
@michelengelen michelengelen merged commit 13a198f into master Apr 14, 2021
@michelengelen michelengelen deleted the MM-29656 branch April 14, 2021 14:59
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Apr 22, 2021
@mm-cloud-bot
Copy link

@michelengelen: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

1 similar comment
@mm-cloud-bot
Copy link

@michelengelen: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry and removed Changelog/Done Required changelog entry has been written labels Apr 22, 2021
chetanyakan added a commit to brightscout-alpha/mattermost-webapp that referenced this pull request Apr 27, 2021
…bapp into MM-34128

* 'MM-34128' of github.com:brightscout-alpha/mattermost-webapp: (104 commits)
  Mm 34578 add subscribe now button (mattermost#7894)
  fix subcription typo in the code (mattermost#7930)
  Fixing keys in Cypress (mattermost#7931)
  fix test on LDAP groups, code theme and incoming webhook (mattermost#7919)
  upgrade cypress to v7.1.0 (mattermost#7923)
  Adding missing Zephyr keys to existing E2E tests (mattermost#7920)
  Prod test fixes (mattermost#7918)
  [MM-11264] Added statuses to channel invite modal (mattermost#7787)
  [MM_20388/ GH_16718] Migrate 'components/signup/signup_email' module and associated tests to TypeScript (mattermost#7699)
  MM-34569 - remove step 3 to guests (mattermost#7838)
  add mfa metadata to specs accordingly (mattermost#7896)
  fix test for forgot password based on new email template (mattermost#7892)
  fix CI run of Cypress and add flag to sort specs to first (mattermost#7913)
  cast to lowercase before comparing (mattermost#7769)
  Migrating a few stragglers from the TS migration (mattermost#7770)
  update tests and keys of search date filters (mattermost#7893)
  [MM-34523] New trial card for license page (mattermost#7826)
  Feature: In-product support for Cloud Trial (mattermost#7907)
  Cypress/E2E: Update tests and keys of search date filters (remaining) (mattermost#7895)
  Mm 29656 (mattermost#7820)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry do-not-merge/release-note-label-needed Docs/Not Needed Does not require documentation
Projects
None yet
10 participants