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.

Sign up for GitHub

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

Jump to bottom

upcoming: [M3-7874] - Linode Create Refactor - Marketplace App Sections #10520

Merged
merged 11 commits into from
Jun 3, 2024
Merged

upcoming: [M3-7874] - Linode Create Refactor - Marketplace App Sections #10520

merged 11 commits into from
Jun 3, 2024

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented May 28, 2024 •
edited
Loading

Description 📝

  • Adds the New apps, Popular apps, and all apps sections to the new Linode Create flow 🆕

Preview 📷

Before After
Screenshot 2024-05-28 at 3 28 15 PM Screenshot 2024-05-28 at 3 28 03 PM
Notice how there are no sections It now looks like the old flow with the proper sections

How to test 🧪

Prerequisites

  • Turn on the Linode Create v2 feature flag using local dev tools

Verification steps

  • Checkout this PR locally or use the internal preview link
  • Go to the Linode Create flow
  • Got to the marketplace tab
  • Verify you see the sections New apps, Popular apps, and all apps
  • Check for parity with production (the old Linode Create flow)
    • Verify the apps are the same under each section

As an Author I have considered 🤔

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@bnussman-akamai bnussman-akamai self-assigned this May 28, 2024
@bnussman-akamai bnussman-akamai marked this pull request as ready for review May 29, 2024 16:09
@bnussman-akamai bnussman-akamai requested a review from a team as a code owner May 29, 2024 16:09
@bnussman-akamai bnussman-akamai requested review from dwiley-akamai, hkhalil-akamai and hana-akamai and removed request for a team May 29, 2024 16:09
bnussman-akamai
bnussman-akamai commented May 29, 2024
View reviewed changes
packages/manager/src/features/OneClickApps/types.ts
Comment on lines +8 to +14
/**
* Set isNew to `true` if you want the app to show up in the "New apps"
* section on the Linode Create flow.
*
* @note this value only affects Linode Create v2
*/
isNew?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be nice because it's going to make maintaining marketplace apps much easier.

I think a feature flag currently determines which apps are considered new

Copy link
Contributor

Choose a reason for hiding this comment

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

If the new apps are rotated frequently, maybe its better to use a feature flag? (So it's not dependent on our release cycle)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is a good point. Before we launch v2, I will make a new flag that will allow us to override anything for emergencies

hkhalil-akamai
hkhalil-akamai approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Code changes are looking clean 🧼 and found no regressions from prod.

packages/manager/src/features/OneClickApps/types.ts
Comment on lines +8 to +14
/**
* Set isNew to `true` if you want the app to show up in the "New apps"
* section on the Linode Create flow.
*
* @note this value only affects Linode Create v2
*/
isNew?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the new apps are rotated frequently, maybe its better to use a feature flag? (So it's not dependent on our release cycle)

@bnussman-akamai bnussman-akamai added Add'tl Approval Needed Waiting on another approval! Marketplace labels May 30, 2024
@jaalah-akamai jaalah-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels May 30, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 30, 2024 •
edited
Loading

Coverage Report: ✅
Base Coverage: 82.34%
Current Coverage: 82.34%

hana-akamai
hana-akamai reviewed May 30, 2024
View reviewed changes
Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Can we also fix the Setup headers for special characters and/or add back the app icon & node notice?

Prod Linode Create v2
image image

@bnussman-akamai bnussman-akamai requested a review from a team as a code owner May 31, 2024 14:53
@bnussman-akamai bnussman-akamai requested review from jdamore-linode and hana-akamai and removed request for a team May 31, 2024 14:53
@bnussman-akamai
Copy link
Member Author

bnussman-akamai commented May 31, 2024 •
edited
Loading

@hana I addressed that feedback!

I also went ahead and deleted the old oneClickApps list and the stackScriptUtils.ts baseApps list. oneClickAppsv2 is now the source of truth for both create flows.

@bnussman-akamai bnussman-akamai added Ready for Review and removed Approved Multiple approvals and ready to merge! labels May 31, 2024
@hana-akamai
Copy link
Contributor

@bnussman looks like search is broken 👀

Screen.Recording.2024-05-31.at.1.45.08.PM.mov

@bnussman-akamai
Copy link
Member Author

bnussman-akamai commented May 31, 2024 •
edited
Loading

@hana-linode Search is not yet implemented in v2, it will come in a later PR. Search should still work on Linode Create v1 though

@hana-akamai hana-akamai added Approved Multiple approvals and ready to merge! and removed Ready for Review labels May 31, 2024
@bnussman-akamai bnussman-akamai merged commit 894afd5 into linode:develop Jun 3, 2024
18 checks passed
@jaalah-akamai jaalah-akamai mentioned this pull request Jun 6, 2024
Merged
@cpathipa cpathipa mentioned this pull request Jun 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hana-akamai hana-akamai hana-akamai approved these changes

@hkhalil-akamai hkhalil-akamai hkhalil-akamai approved these changes

@jaalah-akamai jaalah-akamai jaalah-akamai approved these changes

@dwiley-akamai dwiley-akamai Awaiting requested review from dwiley-akamai dwiley-akamai is a code owner automatically assigned from linode/frontend

@jdamore-linode jdamore-linode Awaiting requested review from jdamore-linode jdamore-linode is a code owner automatically assigned from linode/frontend-sdet

Assignees

@bnussman-akamai bnussman-akamai

Labels
Approved Multiple approvals and ready to merge! Linode Create Refactor Marketplace
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bnussman-akamai @hana-akamai @hkhalil-akamai @jaalah-akamai @bnussman