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

Add cli command modal #727

Conversation

Assem-Hafez
Copy link
Contributor

Summary

Add a modal to show most important cli commands. The commands are grouped into two tabs domain & workflow

Changes

  • Create a reusable component for copying text button
  • Fix the styles of page tabs to add the spacing around the whole element including the endEnhancer
  • Create configs for cli commands
  • Create modal and button components for showing cli commands

Screenshots

Screenshot 2024-11-13 at 16 27 52 Screenshot 2024-11-13 at 16 28 09

Copy link
Member

@adhityamamallan adhityamamallan left a comment

Choose a reason for hiding this comment

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

  1. How do we want to make these commands configurable in the future?
  2. If we want to show the commands on a different page, would it be possible to configure these commands to show there too?

Copy link
Member

Choose a reason for hiding this comment

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

nit: fix the file name (*copy-text-button)

@@ -17,6 +18,7 @@ export default function PageTabs({
setSelectedTab(activeKey);
}}
overrides={overrides.tabs}
endEnhancer={endEnhancer}
Copy link
Member

Choose a reason for hiding this comment

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

Can we call this prop something else? Because individual tabs also have end enhancers, and it can get confusing

Copy link
Contributor

@Assem-Uber Assem-Uber Nov 13, 2024

Choose a reason for hiding this comment

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

Cant think of a name that would make it less confusing tabsEndEnhancer? It still confusing.

Update:
Not the most clear but it is still distinguishable as all individual tabs are configured though the tabs array

@Assem-Uber
Copy link
Contributor

  1. How do we want to make these commands configurable in the future?
  2. If we want to show the commands on a different page, would it be possible to configure these commands to show there too?

1- These commands are already configurable. We have configuration files for tabs and commands that can be replaced.
2- If we are using the same modal/button components we can move them to shared directory.
Otherwise we can create components reading from the same commands configs

@adhityamamallan
Copy link
Member

adhityamamallan commented Nov 13, 2024

These commands are already configurable. We have configuration files for tabs and commands that can be replaced

Sorry, my bad. My question was: how can we prepopulate params in these commands in the future?

@Assem-Uber
Copy link
Contributor

We can use string interpolation.
One option is to add placeholder for values that can be replaced with the values in the page context.
This approach can be even extended to show supported values for each flag on hover.
But keeping this new ideas to the improvement cycle after the Parity with v3

@Assem-Uber Assem-Uber merged commit 2134416 into cadence-workflow:release/4.0.0 Nov 14, 2024
4 checks passed
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.

3 participants