Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Responsive issues on long recipient's name #237

Open
gentlementlegen opened this issue Jun 2, 2024 · 50 comments · May be fixed by #336
Open

Responsive issues on long recipient's name #237

gentlementlegen opened this issue Jun 2, 2024 · 50 comments · May be fixed by #336

Comments

@gentlementlegen
Copy link
Member

When the recipient name is long, it overflows the container and on mobile makes the button going out of the screen. See attached screenshots:

image image

The name should probably scroll sideways or simply be cut on overflow.

@0x4007
Copy link
Member

0x4007 commented Jun 3, 2024

Why does it have the opening bracket? What exactly is the value of the recipient here?

@gentlementlegen
Copy link
Member Author

@0x4007 I am not sure. I recently bought an ENS and that's how it displays for me ever since.

@0x4007
Copy link
Member

0x4007 commented Jun 3, 2024

I broke the ens resolver when I disabled web3 gateway on cloudflare and we could never get it working again.

So yes it renders incorrect data consistently.

For example I own several ens names but one is set at the official one. The official one no longer displays, it just picks the first from the list I own now.

@gentlementlegen
Copy link
Member Author

All right I see, thanks for the info. I believe this is worth fixing for that reason then, and because I guess nothing stops a user to have an ENS like my-super-super-cool-and-long-ens-name-becasue-why-not.eth

@jordan-ae
Copy link
Contributor

/start

Copy link

ubiquibot bot commented Jun 3, 2024

DeadlineMon, Jun 3, 10:30 PM UTC
Registered Wallet 0x2F05fD58023B0a95d1866aa0A3b672cEf05945c5
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@jordan-ae
Copy link
Contributor

/stop

Copy link

ubiquibot bot commented Jul 1, 2024

+ You have been unassigned from the task

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

/start

Copy link

ubiquibot bot commented Jul 5, 2024

Warning! This task was created over 32 days ago. Please confirm that this issue specification is accurate before starting.
DeadlineFri, Jul 5, 8:53 AM UTC
Registered Wallet 0x0BEd00438D57d07E3667b85Fa8EB86Af147C7025
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

@0x4007 Hey, I checked with the same dimensions as mentioned in the issue and It seems like this issue has been resolved already

@jordan-ae
Copy link
Contributor

@Sadaf-A it hasn't been fixed yet. Test it with an ENS. Like this one for example my-super-super-cool-and-long-ens-name-becasue-why-not.eth. Also the sender wallet on the details page has a similar responsiveness glitch.

It seems to work fine with wallet addresses but the styling for ENS domains is not where the glitch is visible.

image

@Keyrxng
Copy link
Contributor

Keyrxng commented Jul 5, 2024

@Sadaf-A The task is ensure that if an ENS name is too long and overflows it's area then it should be gracefully handled.

I would suggest a string manipulation approach personally, as that's how I'd tackle this task for a few reasons. An ENS name can be any length and it'll be hard to account for all of them, also that any CSS changes made will need to be tested to ensure that they do not cause any ripple effects.

Personally, I'd max an ENS display name to the length of an ETH address and add ellipsis if it overflows, ensuring that regardless of length of name the table will not change shape. There is specific formatting currently for addresses that handles ellipsis, we need a similar thing for ENS names but with ... at the end.


You can take any approach you like of course, just be mindful of the knock-on effects of adding/modifying CSS.

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

@jordan-ae @Keyrxng I'm sorry this is my first time contributing to this repo I checked for wallet address but how do I check for ENS?

@Keyrxng
Copy link
Contributor

Keyrxng commented Jul 5, 2024

@Sadaf-A I'd recommend taking a good look at the README and setting everything up, everything you need should be included but if not let me know.

You can simply force a long name for testing in the logic and ensure it's handled effectively.

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

@Sadaf-A I'd recommend taking a good look at the README and setting everything up, everything you need should be included but if not let me know.

You can simply force a long name for testing in the logic and ensure it's handled effectively.

Using README file I'm only able to render wallet addresses it does not mention how to render ENS address

@Keyrxng
Copy link
Contributor

Keyrxng commented Jul 5, 2024

Using README file I'm only able to render wallet addresses it does not mention how to render ENS address

If using anvil account 0 as recommended you would have the ENS name cortex.hiphop. An ENS name is something you buy on-chain and it gives your wallet address an alias, so that's not something we would cover in the the README I'm afraid.

Forcing the conditions will be required for this task unless you have access to a really long ENS name like @gentlementlegen

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

Using README file I'm only able to render wallet addresses it does not mention how to render ENS address

If using anvil account 0 as recommended you would have the ENS name cortex.hiphop. An ENS name is something you buy on-chain and it gives your wallet address an alias, so that's not something we would cover in the the README I'm afraid.

Forcing the conditions will be required for this task unless you have access to a really long ENS name like @gentlementlegen

Thanks, I was able to reproduce it

@0x4007
Copy link
Member

0x4007 commented Jul 5, 2024

Using README file I'm only able to render wallet addresses it does not mention how to render ENS address

If using anvil account 0 as recommended you would have the ENS name cortex.hiphop. An ENS name is something you buy on-chain and it gives your wallet address an alias, so that's not something we would cover in the the README I'm afraid.

Forcing the conditions will be required for this task unless you have access to a really long ENS name like @gentlementlegen

Just edit the DOM to have a long name it takes a second or two to do.

@Keyrxng
Copy link
Contributor

Keyrxng commented Jul 31, 2024

@Sadaf-A I'm just checking on progress here, still working on this?

@ubiquibot ubiquibot bot unassigned Sadaf-A Jul 31, 2024
@Sadaf-A
Copy link

Sadaf-A commented Jul 31, 2024

@Sadaf-A I'm just checking on progress here, still working on this?

My laptop was damaged I had given it for service will continue on this

This was referenced Sep 14, 2024
Copy link
Contributor

ubiquity-os bot commented Sep 19, 2024

@CaptainPrinz, this task has been idle for a while. Please provide an update.

1 similar comment
Copy link
Contributor

ubiquity-os bot commented Sep 19, 2024

@CaptainPrinz, this task has been idle for a while. Please provide an update.

@hydroxylase
Copy link
Contributor

@CaptainPrinz, this task has been idle for a while. Please provide an update.

@Keyrxng has to approve the pull first

@hydroxylase hydroxylase mentioned this issue Sep 26, 2024
Copy link
Contributor

ubiquity-os bot commented Oct 4, 2024

@hydroxylase, this task has been idle for a while. Please provide an update.

@zugdev
Copy link
Contributor

zugdev commented Oct 15, 2024

/start

Copy link
Contributor

ubiquity-os bot commented Oct 15, 2024

Warning! This task was created over 135 days ago. Please confirm that this issue specification is accurate before starting.
Deadline Tue, Oct 15, 7:12 PM UTC
Beneficiary 0xbB689fDAbBfc0ae9102863E011D3f897b079c80F

Tip

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@zugdev zugdev linked a pull request Oct 15, 2024 that will close this issue
@zugdev
Copy link
Contributor

zugdev commented Oct 15, 2024

QA

ens-demo.webm

@0x4007
Copy link
Member

0x4007 commented Oct 16, 2024

It definitely doesn't make sense to truncate the middle when there's plenty of space on the screen

Also QA needs to be posted on the pull

@harshmittal1750
Copy link

/start

Copy link
Contributor

ubiquity-os bot commented Oct 27, 2024

! This issue is already assigned. Please choose another unassigned task.

@0x4007
Copy link
Member

0x4007 commented Oct 27, 2024

@gentlementlegen disqualify isn't working last activity nine days ago

@gentlementlegen
Copy link
Member Author

@0x4007 No it is not working because the SDK is not signing payloads properly for Actions, this was already discussed and there is a PR opened about this: ubiquity-os/ubiquity-os-kernel#168

@0x4007
Copy link
Member

0x4007 commented Oct 28, 2024

Can you merge it

@gentlementlegen
Copy link
Member Author

@0x4007 Merged but the problem is not solved, commented on the PR.

@henalolp
Copy link

henalolp commented Dec 2, 2024

/help

Copy link
Contributor

ubiquity-os bot commented Dec 2, 2024

Available Commands

Command Description Example
/help List all available commands. /help
/start Assign yourself and/or others to the issue/task. /start
/stop Unassign yourself from the issue/task. /stop

@Zomebie
Copy link

Zomebie commented Dec 14, 2024

/start

@gentlementlegen
Copy link
Member Author

The /start command didn't work because the configuration within this repo was pointing to an old version, got fixed.

@Zomebie
Copy link

Zomebie commented Dec 16, 2024

/start

Copy link
Contributor

ubiquity-os bot commented Dec 16, 2024

! This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: Priority: 3 (High), Priority: 4 (Urgent), Priority: 5 (Emergency)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment