-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 New Tab Menu Customization to Settings UI #18015
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH nothing in this PR was a problem for me, although there are some nits. I'm mostly curious how it'll feel in practice (once it's in Canary). 🙂
src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Played around with the branch, it's super cool! Mostly just nits but a couple blockers particularly around the repeated revoking/setting up of the same event handler
}); | ||
|
||
// Clear CurrentFolder to reset the view | ||
_CurrentFolder = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of clearing CurrentFolder
, can we first check whether the previous CurrentFolder
exists and then reset that as the current folder? Similar to the way the ColorSchemesPageVM resets the current scheme if that scheme still exists
Feedback from Bug Bash (11/19)
|
_ScrollToEntry(_ViewModel.RequestAddRemainingProfilesEntry()); | ||
} | ||
|
||
// As a QOL improvement, we scroll to the newly added entry. |
Check failure
Code scanning / check-spelling
Unrecognized Spelling Error
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details. Unrecognized words (1)QOL Previously acknowledged words that are now absentbarbaz Ralph 🫥To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands... in a clone of the [email protected]:microsoft/terminal.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.22/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/11962841195/attempts/1' Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionaryThis includes both expected items (2229) from .github/actions/spelling/expect/alphabet.txt
Consider adding them (in with:
extra_dictionaries:
cspell:cpp/src/lang-jargon.txt
cspell:swift/src/swift.txt
cspell:gaming-terms/dict/gaming-terms.txt
cspell:monkeyc/src/monkeyc_keywords.txt
cspell:cryptocurrencies/cryptocurrencies.txt To stop checking additional dictionaries, add (in check_extra_dictionaries: '' Errors (1)See the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. ✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later. If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 If the flagged items are 🤯 false positivesIf items relate to a ...
|
Summary of the Pull Request
Adds customization for the New Tab Menu to the settings UI.
FolderEntry
FolderEntry
exposesEntries()
(used by the new tab menu to figure out what to actually render) andRawEntries()
(the actual JSON data deserialized into settings model objects). I went ahead and exposedRawEntries()
since we'll need to apply changes to it to then serialize.NewTabMenuViewModel
is the main view model that interacts with the page. It maintains the current view of items and applies changes to the settings model.NewTabMenuEntryViewModel
and all of the other_EntryViewModel
classes are wrappers for the settings model NTM entries.FolderTreeViewEntry
encapsulatesFolderEntryViewModel
. It allows us to construct aTreeView
of just folders.FontIconGlyph
to make this look nice!Verification
Follow-ups