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

feat: replace old trackview icons #17924

Merged

Conversation

pollend
Copy link
Member

@pollend pollend commented May 16, 2024

What does this PR do?

I would like to replace the rest of the track view icons. these are a couple of the icons i could find from EmotionFX and a couple of tweaked ones.
image

How was this PR tested?

Please describe any testing performed.

@pollend pollend changed the title feat: replace old tracview icons feat: replace old track icons May 16, 2024
@pollend pollend changed the title feat: replace old track icons feat: replace old trackview icons May 16, 2024
@byrcolin byrcolin added the sig/content Categorizes an issue or PR as relevant to SIG Content. label May 21, 2024
@lsemp3d
Copy link
Contributor

lsemp3d commented Dec 3, 2024

@pollend I really like this change but it looks like it needs a merge from development to get it to build. Happy to run the build if you can update it

Signed-off-by: Michael Pollind <[email protected]>
@pollend pollend force-pushed the feature/partially-update-track-view-icons branch from 90d02b0 to b0d5894 Compare December 4, 2024 03:11
@pollend
Copy link
Member Author

pollend commented Dec 4, 2024

@lsemp3d I was hoping to get updated icons, from sig-ux.

@lsemp3d
Copy link
Contributor

lsemp3d commented Dec 4, 2024

@lsemp3d I was hoping to get updated icons, from sig-ux.

I think these are a good start

@pollend pollend marked this pull request as ready for review December 4, 2024 03:33
@pollend pollend requested a review from a team as a code owner December 4, 2024 03:33
@AMZN-daimini
Copy link
Contributor

Love this, thanks for looking into it :D

Ideally, instead of adding these to a folder specific to the TrackView, you could give those icons more generic names and add them to the AzQtComponents Images folder: https://github.com/o3de/o3de/tree/development/Code/Framework/AzQtComponents/AzQtComponents/Images

This way they can be re-used across different tools (A "Playback" folder would likely contain most of them). Of course there may be some icons that are very specific to the TrackView workflows, but the common ones should be reusable.

On top of this, are the icons that were replaced still referenced somewhere? Deleting them if they are orphaned would be another small win :)

@lsemp3d
Copy link
Contributor

lsemp3d commented Dec 4, 2024

Love this, thanks for looking into it :D

Ideally, instead of adding these to a folder specific to the TrackView, you could give those icons more generic names and add them to the AzQtComponents Images folder: https://github.com/o3de/o3de/tree/development/Code/Framework/AzQtComponents/AzQtComponents/Images

This way they can be re-used across different tools (A "Playback" folder would likely contain most of them). Of course there may be some icons that are very specific to the TrackView workflows, but the common ones should be reusable.

On top of this, are the icons that were replaced still referenced somewhere? Deleting them if they are orphaned would be another small win :)

@pollend should we hold off on merging to make these changes?

@pollend
Copy link
Member Author

pollend commented Dec 4, 2024

I think I was just following what was already established. All the icons I think are duplicated but I think it makes sense to have like a standard set of icons for consistency.

I also haven't looked at this in a long time so I don't remember which icons I changed. I think I borrowed icons from emotionfx and added my own using inkscape.

@lsemp3d
Copy link
Contributor

lsemp3d commented Dec 4, 2024

I think I was just following what was already established. All the icons I think are duplicated but I think it makes sense to have like a standard set of icons for consistency.

I also haven't looked at this in a long time so I don't remember which icons I changed. I think I borrowed icons from emotionfx and added my own using inkscape.

I can merge and create a new issue with @AMZN-daimini 's suggestions

@lsemp3d lsemp3d merged commit 59f92fd into o3de:development Dec 5, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/content Categorizes an issue or PR as relevant to SIG Content.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants