[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Arrow of NSMenu
From: |
Fred Kiefer |
Subject: |
Re: Arrow of NSMenu |
Date: |
Sat, 6 Apr 2013 23:26:50 +0200 |
Hi Eric,
thank you! This new change looks a lot cleaner and leaves all the mapping logic
inside of NSImage. I think it will be a lot easier to maintain.
Fred
On the road
Am 06.04.2013 um 21:51 schrieb Eric Wasylishen <address@hidden>:
> Hi Fred,
>
> On 2013-04-06, at 9:03 AM, Fred Kiefer <address@hidden> wrote:
>
>> Hi Eric,
>>
>> I don't like this change very much and will try explain why. This does not
>> mean that I doubt the technical correctness of this patch. I just think we
>> should try to find a better solution.
>
> Agreed, it's ugly.
>
>> - First off, I don't really see the issue here. This may be because I don't
>> use themes. But can somebody please explain what would be the problem with
>> using the official names for images in themes? Riccardo already stated that
>> things would work when using the name NSMenuArrow.
>
> If we don't use the nsmappings.strings for themes, themes may have to provide
> a lot of duplicate files (e..g common_3DArrowRight.tiff, NSMenuArrow.tiff)
> with the same contents. Not a huge problem… but it's ugly to have different
> image lookup logic for images inside themes and other images.
>
>> - The big doubt I am having with the change is that now GSTheme has to know
>> about that name mapping which was internal to NSImage up to now.
>> A solution where similar code would be used inside the NSImage method
>> _setImagePath:name: seems a lot cleaner to me. If we build up that reverse
>> map you are using, all the necessary information should be available in that
>> method. I think this would belong into the else case of the if (image !=
>> nil) test you introduced.
>
> I considered that - so +[NSImage _setImagePath:name:] for common_3DArrowRight
> would also set the path for NSMenuArrow and anything else that maps to
> common_3DArrowRight.
>
> There is a corner case with that; if the theme provides both NSMenuArrow and
> common_3DArrowRight, the image used for NSMenuArrow depends on which call to
> +[NSImage _setImagePath:name:] is made first.
>
>> - Another way to get rid of the problem would be to completely remove the
>> name mapping from NSImage. I am a bit reluctant to propose this. That
>> mechanism has been around for a very long time and it allows us to have
>> clearer names. But with the theme code in place this mechanism isn't needed
>> that much any more.
>
> I committed a more radical redesign that is much cleaner, I think.
> Pasting my changelog comment:
>
> I removed the step in theme activation where we call
> +[NSImage _setImagePath:name:] on each image in the theme, and instead
> modified +[NSImage _pathForImageNamed:] to also search the theme images
> directory.
>
> When a GSTheme activates now, it only calls +[NSImage _reloadCachedImages]
> which checks all NSImage cached by name and reloads any whose path has
> changed.
>
> My only worry is whether this will break the GTK or windows themes. IIRC they
> replace the NSImage class and override +imageNamed:, so I think they'll still
> work.
>
> Eric
>
>> Cheers
>> Fred
>>
>> On 06.04.2013 10:06, Eric Wasylishen wrote:
>>> Hi Riccardo,
>>>
>>> I committed a fix in r36474. What I ended up doing is, when a GSTheme
>>> activates, it takes the image name -> path dictionary of images in the
>>> theme, and "expands" it by applying all of the nsmappings.strings mappings.
>>>
>>> So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll
>>> produce a dictionary like:
>>> {
>>> "common_3DArrowRight" : "path/to/common_3DArrowRight.tiff",
>>> "NSMenuArrow" : "path/to/common_3DArrowRight.tiff",
>>> }
>>>
>>> This expanded set of images is then applied to the app state using
>>> +[NSImage _setImagePath:name:], and the same expanded set is unregistered
>>> when the theme deactivates.
>>>
>>> Hope this works for you, and the behaviour sounds sensible.
>>>
>>> Cheers.
>>> Eric
>>>
>>> On 2013-04-05, at 5:25 AM, Riccardo Mottola <address@hidden> wrote:
>>>
>>>> Hi all,
>>>>
>>>> On 03/28/13 16:40, Eric Wasylishen wrote:
>>>>>
>>>>> Hey Riccardo, check the nsmappings.strings file in Images. I think that
>>>>> maps nsmenuarrow to one of the common_ images.
>>>>>
>>>> back to the original problem, which is different from what German supposed.
>>>>
>>>> Let's remember that we have a mapping
>>>>
>>>> NSMenuArrow = common_3DArrowRight;
>>>>
>>>>
>>>> Leaving out Thematic for a moment, I found that placing inside the Theme
>>>> Images a an image named
>>>>
>>>> common_3DArrowRight.tif
>>>>
>>>> doesn't work, while putting one called
>>>>
>>>> NSMenuArrow.tif
>>>>
>>>> works fine and the image gets loaded even dynamically when changing the
>>>> theme.
>>
>>
>> _______________________________________________
>> Gnustep-dev mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/gnustep-dev
>