Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Bohjan/yoroy also talked about not having too much and no duplicate information in tables.
The relationship type table has a lot of columns, can we maybe remove some of these?
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | revamp_types_listing2.patch | 8.29 KB | Berdir |
| #4 | revamp_types_listing.patch | 8.29 KB | Berdir |
Comments
Comment #1
BenK CreditAttribution: BenK commentedDo you mean, for instance, removing the "Plural" column?
Comment #2
BerdirYeah, for example that :)
This was just a thing that I remembered from the UI design patterns session and that table does have a lot of information in it...
Comment #3
BenK CreditAttribution: BenK commentedI'd maybe think about removing the "Implies" column as well.... an admin could edit the relationship type to see what relationships are implied.
--Ben
Comment #4
BerdirOk, what about this...
- Dropped plural name, implications
- Merged approval and expiration column
- Switched to upper case words (like Reciprocal, Yes, No etc.)
- Updated the code to use some 7.x features, updated the hook that other modules use to extend this page and added documentation for that.
Comment #5
BenK CreditAttribution: BenK commentedThis patch looks good. I like all of the choices you made.
Just a few small things:
a) Since everything is now Upper case (which I like), shouldn't the relationship type name also be uppercase? This way, "friend" would be listed as "Friend". I realize that the relationship type edit page only has lowercase versions. So either should we just capitalize it before it is output, or else should we add fields to have "Capitalized singular" and "Capitalized plural" like we do for Userpoints branding? I also think that the relationship type name would look better capitalized in a lot of places (such as the relationship type tabs and listings on user profile pages).
b) I think "One way" should probably be "One-way".
c) In the "Requires approval" column, instead of "Yes (4 days expiration)", how about "Yes (within 4 days)"? I think the meaning is even clearer and we can get rid of a long repetitive word like "expiration".
d) In the "Default relationships" table, do you think the column "Relationships" should appear to the left of the "User" column? This way, the two tables would have the same left-most column. And should that column be renamed "Relationship type"? Also, maybe the second column of this table should vertically align with the second column of the table above. And then the "Operations" column of the two tables could be aligned as well. I just thought this might make things look more orderly.
--Ben
Comment #6
Berdira) Agreed, but we can't just uppercase the first character. This might be different in other languages, etc. So what we could try to do is what Userpoints does, allow to provide upper and lower case variants of singular and plural. But then again, not all languages have the same singular/plural system. Anyway, certainly a separate issue to figure that out.
b) Changed
c) Changed
d) Changed the order, not sure about the theming. I think that will also depend on the used theme and so on. Some themes might for example not be as wide as seven, or not make tables use 100% of the width.
Comment #7
BenK CreditAttribution: BenK commentedHey Berdir,
I agree with all of your points. I tested the latest patch and everything looks good. So I'm marking this RTBC.
As a side note, it occurs to me that it might be nice for new users to have a short description under "Default Relationships" to explain what that section is. My suggestion for text would be:
"Every new user will have the following default relationships upon account creation."
It's not essential, so I'll leave it up to you whether you want to add this to the patch before committing. I'm fine either way.
--Ben
Comment #8
BerdirI'll commit this as is, feel free to open a new issue for that.
One other thing to think about is to deprecated/remove the defaults module in favor of rules integration? Can it do anything that you can't with rules integration? Sounds like that would be the more flexible solution, as you could assign relationships based on some conditions.