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

Modify default InlineKey len to 8 #224

Merged
merged 1 commit into from
Jul 13, 2022
Merged

Conversation

nathanleclaire
Copy link
Contributor

Closes #223

Signed-off-by: Nathan LeClaire nathan@cryptoworkbench.io

@SvenDowideit
Copy link
Member

no?

can we default to not reduced - as that's the normal - we have all the space we need, and reduce it only where its needed for UX - like changes..

@@ -319,7 +319,7 @@ function AccountView(props: { pubKey: string | undefined }) {
<b>ATA</b>
<InlinePK
pk={tAccount.pubkey.toString()}
formatLength={9}
formatLength={8}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 - as in 4chars + '...' + 4 chars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roger

@nathanleclaire
Copy link
Contributor Author

The reason I bring this up actually is because I want to stuff a "Tags" section in that table, ideally for 0.4.0 -- which would contain tags like Faucet, Validator Account, owner=System Program, that type of thing.

@@ -105,7 +105,7 @@ const InlinePK: React.FC<{

InlinePK.defaultProps = {
className: '',
formatLength: 32,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please no, don't minimse by default - its a UX choice that should be only used where space is a constraint.

@nathanleclaire
Copy link
Contributor Author

can we default to not reduced - as that's the normal

Sure

we have all the space we need

Do we? I think we tend to use monitors that are on the larger side. I want ideally WB to work pretty well out of the box on a small display, like a Macbook Air. And for the program changes in particular I think we could get more useful information in there

@SvenDowideit
Copy link
Member

yes - the non-truncated pubkey fits very well in the AccountView.... the table is quite different.

but again - yes, truncate on the changes table, cos that is a space constrained thing.

eventually, it should be responsive, not hard coded...

Signed-off-by: Nathan LeClaire <nathan.leclaire@gmail.com>
@nathanleclaire
Copy link
Contributor Author

OK yea my fault, movin too quick on the original one.

This is now closer to what I had in mind.

Seems WB opens with the left pane quite aggressively expanded, and I always have to drag the right pane over lol -- we'll have to see if we can fix that up

@SvenDowideit
Copy link
Member

OMG. I just realised - we should both think more about this - I'll make a new issue!

LGTM!

Copy link
Member

@SvenDowideit SvenDowideit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nathanleclaire
Copy link
Contributor Author

thx for fast review!

@SvenDowideit SvenDowideit merged commit 43f3509 into main Jul 13, 2022
@SvenDowideit SvenDowideit deleted the be_fussy_about_pubkey_length branch July 13, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why did we go back to full length pubkey displays?
2 participants