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

Update the KBM UI to use a Grid based layout rather than stack panels #2299

Conversation

arjunbalgovind
Copy link
Contributor

@arjunbalgovind arjunbalgovind commented Apr 21, 2020

Summary of the Pull Request

This PR changes the approach of adding the controls on the edit keyboard and edit shortcut window from stack panels to a single grid with three columns. Rows are dynamically added on clicking the + key. The header was also changed to a relative layout. The PR also fixes an indexing bug, where adding and deleting rows in the tables could result in errors on applying the settings.

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • Edit keyboard and Edit shortcut header changed to relative layout
  • 3 column Grid used for all the key/shortcut controls
  • Instead of using a fixed row index, the row index is now calculated based on the index of the control in the grid in the UI. Since these indices are reliable and match what is shown on the UI, the index will always match the buffer vector where the values are stored.
  • Moved drop down selection handlers out of the constructor to a separate function. This was done since all the associated controls may not be initialized at the constructor stage (some stack panels in the parent class would be null).
  • Changed the C++ window logic to account for resizing. Earlier only on the WM_PAINT message the Xaml UI would be resized. However that message is not sent if the window is resized smaller. That has now been fixed.

Screenshots

keyboardManagerDynamic

@arjunbalgovind arjunbalgovind added the Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager label Apr 21, 2020
@arjunbalgovind arjunbalgovind requested review from traies and a team April 21, 2020 20:27
Comment on lines +40 to +63
void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& singleKeyControl, size_t colIndex, std::vector<std::vector<DWORD>>& singleKeyRemapBuffer)
{
dropDown.SelectionChanged([&, table, singleKeyControl, colIndex](winrt::Windows::Foundation::IInspectable const& sender, SelectionChangedEventArgs const& args) {
ComboBox currentDropDown = sender.as<ComboBox>();
int selectedKeyIndex = currentDropDown.SelectedIndex();
// Get row index of the single key control
uint32_t controlIndex;
bool indexFound = table.Children().IndexOf(singleKeyControl, controlIndex);
if (indexFound)
{
int rowIndex = (controlIndex - 2) / 3;
// Check if the element was not found or the index exceeds the known keys
if (selectedKeyIndex != -1 && keyCodeList.size() > selectedKeyIndex)
{
singleKeyRemapBuffer[rowIndex][colIndex] = keyCodeList[selectedKeyIndex];
}
else
{
// Reset to null if the key is not found
singleKeyRemapBuffer[rowIndex][colIndex] = NULL;
}
}
});
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't thing it is a good idea to put the SelectionHandler code for each Dialog here. The SetSelectionHandler method should just receive a callback function and set it as the handler for the SelectionChanged event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem in moving it, is the keyCodeList vector is a member of the KeyDropDownControl class. So I can't define the lambda outside the class and pass the keyCodeList as a callback since that variable does not exist in the parent class.

@arjunbalgovind arjunbalgovind merged commit b5bd2df into microsoft:dev/build-features Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants