Skip to content

Show warning when filename start/end with whitespace in explorer inputbox#92221

Merged
isidorn merged 4 commits intomicrosoft:masterfrom
jeanp413:fix-91187
Mar 11, 2020
Merged

Show warning when filename start/end with whitespace in explorer inputbox#92221
isidorn merged 4 commits intomicrosoft:masterfrom
jeanp413:fix-91187

Conversation

@jeanp413
Copy link
Contributor

@jeanp413 jeanp413 commented Mar 7, 2020

This PR fixes #91187

@isidorn I end up adding a new optional attribute notificationMessage to the IEditableData interface. Let me know what you think.

@isidorn
Copy link
Collaborator

isidorn commented Mar 9, 2020

@jeanp413 thanks for this PR. While this approach makes sense I would like to minimize changes if possible.
I do not see a clear need for a new method notificationMessage, why not just reuse and enhance validationMessage? I think that will make it cleaner and leaner.

@isidorn isidorn added the file-explorer Explorer widget issues label Mar 9, 2020
@jeanp413
Copy link
Contributor Author

jeanp413 commented Mar 9, 2020

I tried that at first but it seemed like more changes at that time 😅
If I update the validationMessage signature to
validationMessage: (value: string) => { content: string, severity: Severity } | null;
I'll need to make some changes in tunnelView.ts

Another option could be string | { content: string, severity: Severity } | null to avoid touching tunnelView.ts

Thoughts?

@isidorn
Copy link
Collaborator

isidorn commented Mar 10, 2020

I think making changes to the tunnelView is perfectly fine, if the changes are for the better.
@alexr00 fyi

@jeanp413
Copy link
Contributor Author

Pushed some changes updating validationMessage return type

Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

The tunnelView changes look good!

@isidorn
Copy link
Collaborator

isidorn commented Mar 11, 2020

Works great, awesome work!
Merging in.

@isidorn isidorn merged commit a17259e into microsoft:master Mar 11, 2020
@isidorn isidorn added this to the March 2020 milestone Mar 11, 2020
@jeanp413 jeanp413 deleted the fix-91187 branch March 11, 2020 17:53
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

file-explorer Explorer widget issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should not allow file or directory names to start with space

3 participants