Potential improvements to address prop to make it more customizable

[JacobHomanics]

Description

Address.tsx updates that allow for better customization of the card while retaining default values that work out of the box.

Address.tsx continued...

Possible usage of customizing the address card in an implementation.

Possible result of the implementation's customizations

Additional Information

• I have read the contributing docs (if this is your first contribution)
• This is not a duplicate of any existing pull request

Your ENS/address: JacobHomanics.eth

[carletex]

Thanks for the idea @Hotmanics

Let us play with it a little bit. A few comments from a first look:

• We should try to standardize with the rest of the components (Balance, etc)
• The prop size is not being used in your changes and it's changing the look of existing components that use a different size. (e.g. TransactionTable.tsx)
• wrapperClasses might be a better name than cardClasses

[JacobHomanics]

@carletex

We should try to standardize with the rest of the components (Balance, etc)

Gotcha, I will begin taking a look at those components.

The prop size is not being used in your changes and it's changing the look of existing components that use a different size. (e.g. TransactionTable.tsx)

This was one of my concerns, so thanks for pointing it out.

You can see that size is also still being used in the Address component.

So I am not sure about the best way to handle this change, especially since it affects other components as well. I will try brainstorming to think of a solution, but I don't have many potential answers at the moment.

wrapperClasses might be a better name than cardClasses

Heard. I will start using that terminology from here on out

---

For reference, this screenshot showcases a real example for the need to have this type of customizability. You can see that the Address component, normally, does not fit into my website's design - making things look out of place and clunky.

This screenshot shows the feature being implemented and allowing the developer to freely customize it to fit into their website's design as the design gets more complex and custom.

[rin-st]

I think it's better to name props textClassName and cardClassName similar to base className prop

[JacobHomanics]

@rin-st @carletex I've implemented ya'lls feedback and made textClasses into textClassName and cardClasses into wrapperClassName.

[rin-st]

@Hotmanics , thanks for your fast response! Some additional points.

I'm thinking do we really need wrapperClassName but still not sure

your example

Possible usage of customizing the address card in an implementation. <img alt="Screenshot 2024-03-11 at 3 46 41 PM" width="687" src="https://private-user-images.githubusercontent.com/32080359/311862530-9511f804-5d9e-4c44-8764-758d05e40722.png

pros

• easier to customize

cons

• all wrapper classNames from your example could be implemented with addional wrapper div when needed
• should it be additional class names since it seems we always need flex items-center? And for adding one class we always need to pass default props + new class, for example flex items-center bg-red-500. Alternative: use wrapperClassName with default classes, not instead of them. flex items-center ${wrapperClassName}

Also noticed that when changing textClassName only text changes, but not avatar and copy icon. So proportions breaks
text-sm

text-4xl

Not sure if it planned behavior or not.

So it looks like these textClassName should be applied some way to avatar and copy/copied icons. Looks like using predefined size like it was before. Or we need also IconsClassName prop 🤯 .

Possible result of the implementation's customizations

For reference, this screenshot showcases a real example for the need to have this type of customizability.

Looks like for both cases you could use just needed wrapper div
tried on se-2 main page (removed wrapper/text classes props):

<div className="flex justify-center items-center space-x-2">
  <p className="my-2 font-medium">Connected Address:</p>
  <div className="rounded-lg text-accent bg-base-300 font-bold">
    <Address address={connectedAddress} />
  </div>
</div>

<div className="flex justify-center rounded-lg py-1 text-accent bg-neutral">
  <Address address={connectedAddress} />
</div>

result:

What do you think?

[JacobHomanics]

What do you think?

@rin-st

I see. I guess my React fundamentals were off point here which may invalidate most of the points of the PR. It never occurred to me that I could just wrap the component into an enclosing div to achieve the same result. That seems moreso the proper way to accomplish the customizability I am seeking. Are there any restrictions using this approach?

I do think the properties of the image, text, and copy button should be customizable and independant of eachother if the developer chooses so.

Gonna work on some commits to achieve that and will ping when I get an example made up.

[JacobHomanics]

Ok...so i think there still may be a need to define the wrapper class as something other than the hardcoded flex items-center as it is still too opinionated IMO. Additionally, as mentioned in my previous comment, it would be nice to independently change the properties of the avatar, text, and copy elements. Thus, the latest commit represents the potential path forward.

There is now BaseAddress and ScaffoldAddress components.

The philosophy is:
BaseAddress acts as the functional component with full customizability.
ScaffoldAddress wraps around BaseAddress, resulting in the functionality and developer workflow of the original Address component. It is opinionated.

Here is an example implementation of both of the components. You can see ScaffoldAddress stays as simple and clean to implement as Address originally desired. Meanwhile, for the extra customizability, you can use BaseAddress directly and pass in the appropriate parameters that are necessary for your project.

This example demonstrates the ability to stack each element and customize them as well if the developer desires that for their application. This was previously not possible. It will be more work and require more understanding on their part, but thats the price of customization IMO.

Thoughts and feedback? @rin-st

[JacobHomanics]

additional adjustments made to make the blockiesize more customizable.

example implementations:

example outcome:

[JacobHomanics]

I've also added for the ability to re-order the components that get rendered

I may be going overboard with these customizations, so let me know if I'm just overwhelming y'all lol.

[rin-st]

Thanks for the changes!

I may be going overboard with these customizations, so let me know if I'm just overwhelming y'all lol.

A little bit 🙂 . Probably it's my bad since I wasn't clear what we need to do. And I thought to discuss first before writing code.

1. wrapperClassNames instead of flex items-center. I think flex items-center is enough for most of the cases. Don't know when and why let's say flex-col needed. Yes, wrapperClassNames could add flexibility, but adding to much freedom causes more bugs in code (both ours and user's) and difficulty of maintaining.
2. Different sizes of avatar/text/icons. Voting for one size prop for all of them. Again, don't know why to use let's say small text and large copy icon at the same time
3. Ordering. Interesting implementation 👍 but again, not sure who needs to change the order.

[JacobHomanics]

Thanks for the changes!

I may be going overboard with these customizations, so let me know if I'm just overwhelming y'all lol.

A little bit 🙂 . Probably it's my bad since I wasn't clear what we need to do. And I thought to discuss first before writing code.

Not your bad at all! My coding style is code first, ask questions later.

1. wrapperClassNames instead of flex items-center. I think flex items-center is enough for most of the cases. Don't know when and why let's say flex-col needed. Yes, wrapperClassNames could add flexibility, but adding to much freedom causes more bugs in code (both ours and user's) and difficulty of maintaining.

I'm sure we can find some scenarios if we think hard enough. The one that comes to mind is the example shown above where they are vertical. For example, you can have a page full of "profiles" in that fashion, rather than a list of, what feels to me, like "widgets". I think user complexity can stay fairly simple if not the same through the introduction of "ScaffoldAddress", which is the component that fits 95% of scenarios (Pretty much what the original "Address" component is). Then, if they want to dig deeper and explore a customizable option, then that option exists. It may not be easier, but they do have the option. Albeit, yes this does add more code management and complexity to the template repository that may not be needed.

2. Different sizes of avatar/text/icons. Voting for one size prop for all of them. Again, don't know why to use let's say small text and large copy icon at the same time

Thus, "ScaffoldAddress". As "Address" currently stands, there is little need to have those options, but once you start allowing customizations, then I can personally see a situation where there is small text and a large copy icon.

3. Ordering. Interesting implementation 👍 but again, not sure who needs to change the order.
   It's a rough and dirty implementation, but it does the job. Glad to clean up later if these changes are something ya'll want to continue to consider.

If it's not clear, I'm obviously for as much customization as possible. I just ask the question "Why not?" The only reason that comes to mind in this situation is that it adds more complexity and management on the Repo's side.

[rin-st]

I agree that the customizing is great!

But for me, mastering is to use minimal possible props that are enough for most cases. And add new props only when required. Like in your case: you need the possibility to update styles - you create pr with the functionality needed.

The only reason that comes to mind in this situation is that it adds more complexity and management

I think it's a big reason :)

Let's wait a bit what others think, but it can take time since part of core contributors are at EthLondon

[technophile-04]

But for me, mastering is to use minimal possible props that are enough for most cases. And add new props only when required. Like in your case: you need the possibility to update styles - you create pr with the functionality needed.

++

---

Thanks so much @Hotmanics for this !! Just created #786 out of this discussion, I think it's better if we first discuss their and then tackle this. Since then will have a better picture and also in line with considering other components 🙌

Really feel bad for closing this :( and appreciate the hard work but that's why we have this note while creating the PR :

But thanks again for contribution!! This led us to #786 and we could totally re-open this after #786 if we feel this is the way to go 🙌!