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(Templates): Support auto selecting template #6498

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Elaina-Lee
Copy link
Contributor

@Elaina-Lee Elaina-Lee commented Jan 28, 2025

Type of Change

  • Bug fix
  • Feature
  • Other

New Behavior

Add support to auto selecting template. When this new parameter is set, the template designer will automatically select the template as if user clicked the template and create.

Impact of Change

This is additive change so it's fully back compatible
built on top of #6485 with additional changes

Test Plan

Tested locally and added CIT

Screenshots or Videos (if applicable)

image

@Elaina-Lee Elaina-Lee marked this pull request as ready for review January 28, 2025 21:28
@Elaina-Lee Elaina-Lee requested a review from hartra344 as a code owner January 28, 2025 21:28
Comment on lines +40 to +46
const DataProviderInner = ({
customTemplates,
isConsumption,
existingWorkflowName,
viewTemplate,
children,
}: TemplatesDataProviderProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we be loading all templates data when the ask from portal blade is to only open one single template? This is unnecessarily going to increase the panel load time.. that is why I was saying why do you want to lock the template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are looking for the least amount of code change for this purpose + the background of the template list view.....
I understand that we discussed about the concern of locking the template, but we had multiple meetings involving Divya where she agreed to the use case of this having to be the case..
@tonytang-microsoft-com would you have thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this would be the least we could do to align with the current template experience. If we load only one template, then the UX will only have 1 template shown in the background, which looks different. I don't think this is a blocker and we could optimize the performance later if needed IMHO.

@@ -214,6 +215,6 @@ export const reviewCreateTab = (
}
}
},
secondaryButtonDisabled: isCreating,
secondaryButtonDisabled: isCreating || (!previousTabId && isTemplateNameLocked),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have this logic in all secondary buttons related to template locked? in reviewCreateTab previousTabId is always true right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previousTabId shouldn't be present in case of consumption template with no connections/parameters

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