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

[IMPROVE] Move app list install logic to app menu #27056

Merged
merged 19 commits into from
Nov 11, 2022

Conversation

rique223
Copy link
Contributor

@rique223 rique223 commented Oct 11, 2022

Proposed changes (including videos or screenshots)

Jira task: MKP-136

Moved the install/buy/subscribe logic from the CTA buttons that existed in the app list to the app menu. Now the user can install apps by clicking in the kebab menu in the right of the app list entry. Also solved a little undefined bug happening in the app permissions modal.
Demo gif:
app_menu

Issue(s)

Steps to test or reproduce

Further comments

Moved the install/buy/subscribe logic from the CTA buttons that existed in the app list to the app menu. Now the user can install apps by clicking in the kebab menu in the right of the app list entry. Also solved a little undefindd bug happening with the app permissions modal.
@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #27056 (03437a7) into develop (9aec545) will increase coverage by 0.78%.
The diff coverage is n/a.

❗ Current head 03437a7 differs from pull request most recent head 7c8f39c. Consider uploading reports for the commit 7c8f39c to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27056      +/-   ##
===========================================
+ Coverage    40.93%   41.72%   +0.78%     
===========================================
  Files          851      826      -25     
  Lines        18700    18233     -467     
  Branches      2050     1988      -62     
===========================================
- Hits          7655     7607      -48     
+ Misses       10760    10340     -420     
- Partials       285      286       +1     
Flag Coverage Δ
e2e 41.72% <ø> (+0.78%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@RocketChat RocketChat deleted a comment from lgtm-com bot Oct 13, 2022
@rique223 rique223 marked this pull request as ready for review October 14, 2022 03:29
@rique223 rique223 requested a review from a team as a code owner October 14, 2022 03:29
@rique223 rique223 marked this pull request as draft October 31, 2022 14:01
@rique223 rique223 marked this pull request as ready for review November 1, 2022 17:36
@rique223 rique223 added this to the 5.4.0 milestone Nov 4, 2022
Applied DRY to two helper functions of both AppStatus and AppMenu in order to improve maintainability.
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Nov 10, 2022
@ggazzo ggazzo merged commit e316d3c into develop Nov 11, 2022
@ggazzo ggazzo deleted the improve/marketplace-apps-buttons branch November 11, 2022 13:15
bkrith pushed a commit to bkrith/Rocket.Chat that referenced this pull request Nov 12, 2022
@ggazzo ggazzo mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad: apps ecosystem AECO stat: QA tested stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants