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

feat(feature flags): add plain UI for Feature Flag copy #18324

Merged
merged 8 commits into from
Nov 2, 2023

Conversation

jurajmajerik
Copy link
Contributor

feature flag: multi-project-feature-flags (rolled out to #team-feature-success only)

Changes

Added plain UI for copying a feature flag from one project to another.

Added a new Projects tab within the Feature Flag view. Put everything there as it feels cleaner / don't want to clutter the Overview. We can adjust as we go.

image

How did you test this code?

👀

@jurajmajerik jurajmajerik requested review from neilkakkar, liyiy and smallbrownbike and removed request for smallbrownbike November 1, 2023 14:32
Copy link
Contributor

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

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

so far so good 👀

frontend/src/scenes/feature-flags/FeatureFlagProjects.tsx Outdated Show resolved Hide resolved
columns={columns}
emptyState="This feature flag is not being used in any other project."
/>
<CopyFeatureFlagModal />
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is on it's own page, a modal doesn't make too much sense to me, can we inline it instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

weak suggestion^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, that makes a lot of sense :)

</div>
<Field name="Project" label="Project" className="flex-1">
<LemonSelect
value={currentOrganization?.teams?.[0].id}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the current team?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is just a placeholder for now. The whole tab will be hidden for anyone having only 1 team and here, some other team than the current team can be pre-selected (or none)

>
<div className="max-w-120">
<Form
logic={featureFlagLogic}
Copy link
Contributor

Choose a reason for hiding this comment

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

depending on whether there's a bindlogic before the tabs or not, this might require the props to be passed in 👀

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@@ -16,6 +16,7 @@ export enum FeatureFlagsTab {
Analysis = 'analysis',
USAGE = 'usage',
PERMISSIONS = 'permissions',
PROJECTS = 'projects',
Copy link
Contributor

Choose a reason for hiding this comment

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

We can change this later on but I think 'projects' makes me think of feature flag projects (??) when the concept we want more is like... feature flag environments xD or like... feature flag syncs...

Copy link
Contributor

Choose a reason for hiding this comment

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

idk, non blocking right now, but it'll be confusing later on IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, on the other hand users already use projects as environments, so a discrepancy in naming might be confusing here. But happy to discuss it, we can always adjust.

Copy link
Contributor

@liyiy liyiy left a comment

Choose a reason for hiding this comment

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

I assume you want to get this UI boilerplate in so it's easier to add the rest, so LGTM!

@jurajmajerik
Copy link
Contributor Author

Thanks for the review @liyiy, in the meantime I moved the form out of the modal - no big changes otherwise, so will merge now to keep moving :)

image

@jurajmajerik jurajmajerik enabled auto-merge (squash) November 2, 2023 20:49
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@jurajmajerik jurajmajerik merged commit f8b5af1 into master Nov 2, 2023
71 of 72 checks passed
@jurajmajerik jurajmajerik deleted the fix/feature-flags-filters branch November 2, 2023 21:28
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.

4 participants