-
Notifications
You must be signed in to change notification settings - Fork 34.7k
use uuid and try dynamic titles #94686
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
Conversation
pinnedAction.setActivity(activity); | ||
} | ||
} | ||
|
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.
Is not this change my viewlet icon every time I rearrange my views in the viewlet? I would recommend to have dynamic title and icons only for generated view containers and only gets updated when its associated view gets moved.
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.
I only included this code as a test as I thought you might have better expertise on the title/icon stuff. However, I still am not convinced we should treat built-in views differently that generated ones. Search without the search view is not Search to the user.
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.
I updated the heuristic to use the icon of the container if:
- we have no icons from views (custom case for extension views)
- we have a view with the same icon as the container (e.g. so long as search is in search, it will not change its icon)
This works in most cases but we still have an issue where the composite bar's representation (title in panel or icon in activity bar) is not accurate until the view is opened as it must use the static registration until the view is opened. This is similar to the issue we had with the debug console header in panel.
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.
Yeah. Will do some thinking and look into this.
This PR: