Fix camera device icon in hwmanager and other places. #58

Merged
MicheleC merged 1 commits from fix/camera into master 4 years ago
Chris commented 4 years ago
Collaborator

Introduced by commit fee189b236 back in time.

Signed-off-by: Chris xchrisx@uber.space

Introduced by commit fee189b236 back in time. Signed-off-by: Chris <xchrisx@uber.space>
Owner

The problem seems to have come with commit 11935291c7 and therefore only concerns the master branch.

At the same time, the question is whether file names should be camera-photo-mounted and camera-photo-umounted because camera-photo seems to be the basis used in other themes.

The problem seems to have come with commit 11935291c7 and therefore only concerns the master branch. At the same time, the question is whether file names should be `camera-photo-mounted` and `camera-photo-umounted` because `camera-photo` seems to be the basis used in other themes.
Chris commented 4 years ago
Poster
Collaborator

Interesting. Question would be, if that would interact with other places using that right name. The mediamanager, for example. So I think we can merge that now, if you are okay with it and that is open for later.

But, if I understand you right: With that change users using the classic theme for example, would have no icon as result now?

Interesting. Question would be, if that would interact with other places using that right name. The mediamanager, for example. So I think we can merge that now, if you are okay with it and that is open for later. But, if I understand you right: With that change users using the classic theme for example, would have no icon as result now?
Ghost commented 4 years ago

Does all this relates to bug 2675 in some ways or others?

Does all this relates to bug 2675 in some ways or others?
Chris commented 4 years ago
Poster
Collaborator

No, but it's interesting, because after reading about that bug, I have realized that I came about that code also and I think there is a test if some device holds some DCIM folder, which be some orientation, if the phone is using a camera icon or so. I am not sure anymore exactly, but that might be related. And yes, there is also a phone icon, which could be used. But that's another topic. This here was just mainly about the "kcmhwmanager", which is only existing in the master branch. There, you have a device listing, but the camera icon here had no icon. But seems it's not that simple as I first thought. We can discuss and think about that further. 😄

No, but it's interesting, because after reading about that bug, I have realized that I came about that code also and I think there is a test if some device holds some DCIM folder, which be some orientation, if the phone is using a camera icon or so. I am not sure anymore exactly, but that might be related. And yes, there is also a phone icon, which could be used. But that's another topic. This here was just mainly about the "kcmhwmanager", which is only existing in the master branch. There, you have a device listing, but the camera icon here had no icon. But seems it's not that simple as I first thought. We can discuss and think about that further. :smile:
SlavekB added this to the R14.1.0 release milestone 4 years ago
Owner

I don't think this PR is required.

camera-photo is for generic camera devices and the icon name is standard and available in other icon sets too.

camera-mounted and camera-unmounted are used when accessing camera storage and they refer to different icons.

camera-photo is used when getting info for a generic camera device.

camera-mounted/unmounted when referring to storage devices in particular, which are a specialization of a generic device.

Therefore in my opinion this PR should be marked invalid.

I don't think this PR is required. camera-photo is for generic camera devices and the icon name is standard and available in other icon sets too. camera-mounted and camera-unmounted are used when accessing camera storage and they refer to different icons. camera-photo is used when getting info for a generic camera device.<br/> camera-mounted/unmounted when referring to storage devices in particular, which are a specialization of a generic device. Therefore in my opinion this PR should be marked invalid.
Owner

Looked in more detail into this and discussed with Slavek.

camera-photo is in tdegraphics and tdeartwork. It is used in tdelibs inside tdehw. The correct way would be to keep the name "camera-photo" and move camera-photo from tdegraphics to tdelibs.

@Chris: ok for you to do that?

Looked in more detail into this and discussed with Slavek. camera-photo is in tdegraphics and tdeartwork. It is used in tdelibs inside tdehw. The correct way would be to keep the name "camera-photo" and move camera-photo from tdegraphics to tdelibs. @Chris: ok for you to do that?
Chris commented 4 years ago
Poster
Collaborator

@MicheleC, sure, I can do that. I just noticed some missing icon in the hwmanager and expected that to be introduced by the pointed out series of commits back in time, because there were other places where that was really the case. But after looking on what Slavek told me and what you pointed out, that is logical. 👍

@MicheleC, sure, I can do that. I just noticed some missing icon in the hwmanager and expected that to be introduced by the pointed out series of commits back in time, because there were other places where that was really the case. But after looking on what Slavek told me and what you pointed out, that is logical. :+1:
Owner

Thanks @Chris. Should I mark this PR invalid and close it and then you create new ones for moving the camera-photo icons from tdegraphics to tdelibs? Or do you prefer to use this PR and rework your commit?

Thanks @Chris. Should I mark this PR invalid and close it and then you create new ones for moving the camera-photo icons from tdegraphics to tdelibs? Or do you prefer to use this PR and rework your commit?
Chris commented 4 years ago
Poster
Collaborator

@MicheleC, I will re-use this one here, but at the moment I don't have much time.

@MicheleC, I will re-use this one here, but at the moment I don't have much time.
Chris commented 4 years ago
Poster
Collaborator

@MicheleC, I noticed that there is also filesys-camera-photo in tdegraphics. But that are the same icons. So the question is if to keep that in tdegraphics or just to move it to tdelibs too, or also just to use device-camera-photo in all that places, because filesys-camera-photo is not really needed at all? 😄

@MicheleC, I noticed that there is also *filesys-camera-photo* in tdegraphics. But that are the same icons. So the question is if to keep that in tdegraphics or just to move it to tdelibs too, or also just to use *device-camera-photo* in all that places, because *filesys-camera-photo* is not really needed at all? :smile:
Owner

filesys-camera-photo does not seem to get install in debian. Probably safe to remove it all together??

filesys-camera-photo does not seem to get install in debian. Probably safe to remove it all together??
Chris commented 4 years ago
Poster
Collaborator

@MicheleC, sure, how you like. 👍

If Slavek is also okay about that, I will do that. Maybe that was just something which was in planing back in time to use, but was never done.

@MicheleC, sure, how you like. :+1: If Slavek is also okay about that, I will do that. Maybe that was just something which was in planing back in time to use, but was never done.
Owner

Yes, if these icons aren't installed, then there's probably no reason to keep them here. You can possibly check if they are installed in RPM packages.

Yes, if these icons aren't installed, then there's probably no reason to keep them here. You can possibly check if they are installed in RPM packages.
Chris commented 4 years ago
Poster
Collaborator

That should be okay this way.

That should be okay this way.
MicheleC closed this pull request 4 years ago
MicheleC deleted branch fix/camera 4 years ago
MicheleC modified the milestone from R14.1.0 release to R14.0.8 release 4 years ago
The pull request has been merged as d4845ced49.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tdelibs#58
Loading…
There is no content yet.