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

Disable entrypoint checkbox in Project Files view #2487

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

dotNomad
Copy link
Collaborator

@dotNomad dotNomad commented Dec 12, 2024

This prevents the user from unchecking the entrypoint checkbox (and therefor removing the entrypoint from the files attribute in their configuration).

The configuration can still be edited by hand, but the sidebar no longer allows getting into this bad state.

Intent

Part of #1493

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

User Impact

The user will no longer be able to uncheck the entrypoint's checkbox.

Directions for Reviewers

Ensure that the entrypoint, no matter the details in the configuration, can be unchecked.

Checking the entrypoint checkbox should still be available if the configuration is manually edited and the entrypoint needs to be added.

@dotNomad dotNomad force-pushed the dotnomad/disable-entrypoint-checkbox branch from f9a03ca to c6c7552 Compare December 12, 2024 23:47
@dotNomad dotNomad marked this pull request as ready for review December 12, 2024 23:48
Copy link
Collaborator

@sagerb sagerb left a comment

Choose a reason for hiding this comment

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

Code looks great. I'll test shortly.

Copy link
Collaborator

@sagerb sagerb left a comment

Choose a reason for hiding this comment

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

Code functions well. I tested it with multiple existing configurations.

I do wonder if the entrypoint file should be visually indicated a little more than the slight opacity change which you implemented. Perhaps with another visual icon, asterick, something?

I do think, regardless of the visual note above, that you should go ahead and add to the tooltip that the entrypoint cannot be removed from the deployment file list.

@dotNomad
Copy link
Collaborator Author

I do wonder if the entrypoint file should be visually indicated a little more than the slight opacity change which you implemented. Perhaps with another visual icon, asterick, something?

The entrypoint file should be emphasized with the color, and the opacity change is to avoid the disabled state making it semi-transparent (and less emphasized). We don't have any other icons or anything in that Project Files view so I'm not quite sure what exactly to do to further visually emphasize it. 🤔

I do think, regardless of the visual note above, that you should go ahead and add to the tooltip that the entrypoint cannot be removed from the deployment file list.

Great point. I'll get that in real quick ⌨️ 🏃

@dotNomad dotNomad force-pushed the dotnomad/disable-entrypoint-checkbox branch from c6c7552 to 23230eb Compare December 21, 2024 00:00
@dotNomad dotNomad requested a review from sagerb December 21, 2024 00:00
@dotNomad
Copy link
Collaborator Author

dotNomad commented Dec 21, 2024

Got the tooltip change in 😄

@dotNomad dotNomad force-pushed the dotnomad/disable-entrypoint-checkbox branch from 45632a9 to 39ad424 Compare December 21, 2024 00:17
@dotNomad dotNomad force-pushed the dotnomad/disable-entrypoint-checkbox branch from 39ad424 to 976e62b Compare December 21, 2024 00:19
@sagerb
Copy link
Collaborator

sagerb commented Dec 21, 2024

Thanks for making the changes. @marcosnav I haven't verified the functionality after Jordan made the last changes. Can you test and merge?

Copy link
Collaborator

@marcosnav marcosnav left a comment

Choose a reason for hiding this comment

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

Confirmed latest changes

Screen Shot 2024-12-23 at 10 56 42

@marcosnav marcosnav merged commit f909de6 into main Dec 23, 2024
14 checks passed
@marcosnav marcosnav deleted the dotnomad/disable-entrypoint-checkbox branch December 23, 2024 01:58
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