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

Cc 7146/convert consul hcp to a simpler component for some upcoming changes #20344

Merged
merged 8 commits into from
Jan 27, 2024
Merged

Cc 7146/convert consul hcp to a simpler component for some upcoming changes #20344

merged 8 commits into from
Jan 27, 2024

Conversation

chris-hut
Copy link
Contributor

@chris-hut chris-hut commented Jan 25, 2024 •
edited by jira bot
Loading

Description

As part of CC-7146 we'll be adding a sidebar item about the cluster's link status and need to modify the logic from the existing Back to Consul Nav bar item.

This PR refactors the component that renders the Back to consul item in the following ways:

  • Moves it to the consul-ui package, for easier use and editing
  • Kills the now empty consul-hcp package to clean up a little bit

Testing & Reproduction steps

  • yarn start
  • In your browser's dev tools run the following to set/unset the following ENV variables:
    • To enable: Scenario('CONSUL_HCP_ENABLE=1'); and Scenario('CONSUL_HCP_URL=https://corn.com')
    • To disable: delete them from your cookies
  • When enabled, you should see the classic <- Back to HCP nav bar item
  • When disabled, that will be hidden

image

Links

PR Checklist

  • updated test coverage

References

image

@chris-hut chris-hut added theme/ui Anything related to the UI backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. pr/no-changelog PR does not need a corresponding .changelog entry labels Jan 25, 2024
@WenInCode
Copy link
Contributor

@hut just taking a look now.

Is there a reason this is backported to 1.17 and not just applied to the new 1.18 release? I haven't followed past PRs so apologies if this works was always planned to be backported to 1.17.x

WenInCode
WenInCode approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@WenInCode WenInCode left a comment

Choose a reason for hiding this comment

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

LGTM, question regarding license and backporting aside.

ui/packages/consul-ui/app/components/hcp-nav-item/index.js Show resolved Hide resolved
WenInCode
WenInCode reviewed Jan 25, 2024
View reviewed changes
Copy link
Contributor

@WenInCode WenInCode left a comment

Choose a reason for hiding this comment

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

I think you may want to also remove this bit?

consul/ui/packages/consul-ui/lib/startup/templates/body.html.js

Lines 111 to 114 in f42152a

'CONSUL_HCP_ENABLE': {
name: 'consul-hcp',
default: ${config.operatorConfig.HCPEnabled}
}

Honestly, confusing. It looks like a IIFE that is called with an object of the features/packages that should be checked if they need to be included but we are also manually doing that above? I'm thinking it may be something that runs for either configuration enabled or if they are enabled by env vars? Not sure right now.

@chris-hut
Merge branch 'main' of github.com:hashicorp/consul into CC-7146/conve…
9a08c0b 
…rt-consul-hcp-to-a-simpler-component-for-some-upcoming-changes
@chris-hut chris-hut mentioned this pull request Jan 26, 2024
Merged
@chris-hut chris-hut merged commit 75af5b1 into main Jan 27, 2024
59 checks passed
@chris-hut chris-hut deleted the CC-7146/convert-consul-hcp-to-a-simpler-component-for-some-upcoming-changes branch January 27, 2024 00:39
@hc-github-team-consul-core hc-github-team-consul-core mentioned this pull request Jan 27, 2024
Closed
1 task
@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

2 similar comments
@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@chris-hut chris-hut mentioned this pull request Jan 29, 2024
Merged
@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

4 similar comments
@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

8 similar comments
@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@chris-hut, a backport is missing for this PR [20344] for versions [1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@chris-hut chris-hut added pr/no-backport and removed backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. labels Feb 12, 2024
@chris-hut chris-hut mentioned this pull request Mar 11, 2024
Merged
This was referenced Sep 26, 2024
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WenInCode WenInCode WenInCode approved these changes

Assignees
No one assigned
Labels
pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry theme/ui Anything related to the UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-hut @WenInCode @hc-github-team-consul-core