Skip to content

Conversation

@samgst-amazon
Copy link
Contributor

@samgst-amazon samgst-amazon commented Oct 3, 2024

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Description

Updated ActionToolbar's layout strategy to ensure no wrapping or auto-hide occurs when components don't fit into the toolbar. This change also replaces the Deprecated layoutPolicy method.

OLD BEHAVIOR:
oldDropdowngif

NEW BEHAVIOR:
newDropdowngif

Checklist

  • My code follows the code style of this project
  • I have added tests to cover my changes
  • A short description of the change has been added to the CHANGELOG if the change is customer-facing in the IDE.
  • I have added metrics for my changes (if required)

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

@samgst-amazon samgst-amazon requested a review from a team as a code owner October 3, 2024 17:31
@samgst-amazon samgst-amazon marked this pull request as draft October 3, 2024 17:36
@samgst-amazon
Copy link
Contributor Author

samgst-amazon commented Oct 3, 2024

Okay, this should be a 1 line fix but I am running into some versioning issues. The current line that we have that sets the layout policy uses a deprecated function that as of 2024.1 does not work(the documentation says this does nothing)

Replacing that with the new LayoutStrategy (instead of policy) works for the new versions, but does not compile for 2023.3 since it cannot resolve the references to the new ToolbarLayout Class in the import statement.

layoutPolicy -- old and only works for 2023.3 <=. No effect for new versions, and the default behavior seems to have changed from NOWRAP to AUTO

layoutStrategy -- new method for layout. seems to default to AUTO for instead of NOWRAP, works for new versions, doesn't compile for old versions since it imports a new package only available 2024.1 >=

Added in some logic to check the version, since importing and referencing com.intellij.openapi.actionSystem.toolbarLayout in old versions seems to break it.

I think once 2024.3 releases and we stop supporting 2023.3 we can change this to one single line:

layoutStrategy = ToolbarLayoutStrategy.NOWRAP_STRATEGY
Screenshot 2024-10-03 at 10 54 57 AM
Screenshot 2024-10-03 at 10 55 20 AM

@samgst-amazon samgst-amazon marked this pull request as ready for review October 3, 2024 19:19
…own' into samgst/FixToolkitConnectionDropdown

# Conflicts:
#	plugins/toolkit/jetbrains-core/src/software/aws/toolkits/jetbrains/core/explorer/AwsToolkitExplorerToolWindow.kt
Copy link
Contributor

@manodnyab manodnyab left a comment

Choose a reason for hiding this comment

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

Can you please add a screenshot of the new dropdown to the description of the PR?

Comment on lines 16 to 18
ActionManager.getInstance().createActionToolbar(ActionPlaces.TOOLBAR, group, true).apply {
layoutStrategy = ToolbarLayoutStrategy.NOWRAP_STRATEGY
setTargetComponent(toolWindow)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since only this section differs across the versions, can we move the creation of this component to the util file and retain the creation for the panel in the init block of AWSToolkitExplorerToolWindow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & done, Thanks for the feedback :)


import com.intellij.openapi.actionSystem.ActionToolbar

object ActionToolbarLayoutUtil {
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably dont need this object, it can be a standalone function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it

…own' into samgst/FixToolkitConnectionDropdown

# Conflicts:
#	plugins/toolkit/jetbrains-core/src-233/software/aws/toolkits/jetbrains/core/explorer/ActionToolbarLayoutUtil.kt
#	plugins/toolkit/jetbrains-core/src-241+/software/aws/toolkits/jetbrains/core/explorer/ActionToolbarLayoutUtil.kt
#	plugins/toolkit/jetbrains-core/src/software/aws/toolkits/jetbrains/core/explorer/AwsToolkitExplorerToolWindow.kt
Comment on lines +3 to +9
package software.aws.toolkits.jetbrains.core.explorer

import com.intellij.openapi.actionSystem.ActionToolbar

fun setToolbarLayout(toolbar: ActionToolbar) {
toolbar.layoutPolicy = ActionToolbar.NOWRAP_LAYOUT_POLICY
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ToolbarLayoutStrategy doesn't exist in 233, so it would be less overall work if we just aliased
ToolbarLayoutStrategy.NOWRAP_STRATEGY to be ActionToolbar.NOWRAP_LAYOUT_POLICY i n233

@rli rli merged commit 5c91df6 into main Oct 10, 2024
11 of 15 checks passed
@rli rli deleted the samgst/FixToolkitConnectionDropdown branch October 10, 2024 17:50
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