Closed Bug 1335050 Opened 4 years ago Closed 4 years ago
Allow customization of object rep's title generation
At the moment reps/object's getTitle method is too opinionated where title depends on on whether the object has class property. In some cases it is not desired so it would be nice to be able to pass getTitle as part of props.
Component: Developer Tools → Developer Tools: Shared Components
I like the patch. Do you have any use case for the new property? Shouldn't we do it for all reps? Honza
I am also CCing Julian here. Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #2) > I like the patch. Do you have any use case for the new property? Shouldn't > we do it for all reps? > > Honza Hi Jan, so my use case is for the work (still not in any of the review queues) that I'm doing for accessibility inspection. I am rendering a tree with accessibility object properties where one of them is called attributes. They correspond to what accessibility module calculates for any particular accessible object. One of the attribute names that it exposes is "class". So what happens is when I want to use an Obj rep, it uses "class" attribute's value as the title. That is however confusing since non of the accessibility attributes have any priority or elevated importance that could be used as a title. What my override getTitle prop does is returning "Object" string (similar to what obj does by default when class is not present). Hope this is enough context. Not sure what other ones you have in mind, my guess is that it could be useful for other opinionated ones, but I, so far, haven't seen any issues in my use cases other than Obj.
Just a heads up: the reps are in the process of being moved to Github. I should complete the migration this week. Feel free to finish the development/review here, I'll port the development to Github in case this doesn't land before the migration.
:yzen, do note that we already do such thing in the Grip reps (https://github.com/devtools-html/devtools-reps/blob/master/src/reps/grip.js#L33), where we only pass a string, and not a function. Like Honza said, it would be really nice to have such mechanism for all Reps :)
@yzen: would it be enough for you to use the same approach as Grip rep? (see comment #6) So, getting the title would look like as follows: let title = this.props.title || object.class || "Object"; ... so, you can just pass an extra 'title' prop to customize the title. This way the implementation would be consistent between Grip and Obj reps. Honza
Making implementation consistent between Grip and Obj
Comment on attachment 8834748 [details] [diff] [review] 1335050 patch v2 Review of attachment 8834748 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch! R+ assuming Try is green (note that the condition checking whether the `object` is null is gone now) Honza
Attachment #8834748 - Flags: review?(odvarko) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb66941fca47 Allow customization of object rep's title generation. r=Honza
https://hg.mozilla.org/mozilla-central/diff/fb66941fca47/git is an empty file. Is this intentional?
My bad! my git-hg tool added this somehow. Here's the patch to get rid of it.
Attachment #8837608 - Flags: review?(odvarko)
Comment on attachment 8837608 [details] [diff] [review] 1335050 clean up Thanks for working on this! Honza
Attachment #8837608 - Flags: review?(odvarko) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e74396a79be cleaning up acceidentally committed file. r=Honza
You need to log in before you can comment on or make changes to this bug.