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

Update Guardian Admin UI to match Figma spec #168

Merged
merged 18 commits into from
Sep 2, 2023

Conversation

wbobeirne
Copy link
Collaborator

@wbobeirne wbobeirne commented Aug 31, 2023

Closes #166, closes #64, closes #95 (sorta?), closes #27, closes #148

Apologies for the size of this PR. Doing it all at once made it easier to get the organization and component abstractions right the first time, rather than having to re-work it a piece at a time.

What This Does

  • Updates shared UI components to match figma spec and adds new reusable components
    • Chakra's Card component now matches Figma spec
    • Custom Table component now matches Figma spec
    • Custom CopyInput component now matches Figma spec
      • Added new sm size support
      • <Input readOnly /> now renders with lighter text
    • Custom Wrapper component now matches Figma spec
      • Added new size="md|lg" prop for admin's wider 1200px template
    • Added new StatusIndicator component
      • This used to be Pill in guardian-ui, but no longer comes with the label prefix
    • Added new KeyValues component for displaying lists of information with a label on top and a value below
  • Updated guardian UI admin to match Figma spec
  • Lightly tweaked gateway UI to be closer to Figma spec
    • Ellipsized node id
    • <Wrapper size="lg"> to match Figma spec width
    • Refactored to use new Card styles
    • Gateway UI is still not 1:1 with Figma spec

Note that some deviations to the Figma spec were made to match the information we have available:

  • Federation Info's "Consensus version" is a single digit, not a major.minor
  • Bitcoin Node just shows "Unknown" until we get a way to fetch non-encoded module configs
  • Guardian table's "Last contribution timestamp" was replaced with "Last contribution epoch"
  • Gateway table's "Name" was replaced with "Node ID" due to not having the alias
  • Gateway table has an additional "Gateway ID" column
  • Gateway table's "Incoming fee" and "Outgoing fee" were replaced with a single "Gateway fee"

Screenshots

Guardian UI admin

Screenshot 2023-08-31 at 11 58 59 AM

Gateway UI

Screenshot 2023-08-31 at 12 16 25 PM

Copy link
Collaborator

@EthnTuttle EthnTuttle left a comment

Choose a reason for hiding this comment

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

Not finished but left some comments.

Still need to review:

  • apps/guardian-ui/src/admin/FederationAdmin.tsx
  • apps/guardian-ui/src/components/BalanceCard.tsx
  • apps/guardian-ui/src/components/BitcoinNodeCard.tsx
  • apps/guardian-ui/src/components/FederationInfoCard.tsx
  • apps/guardian-ui/src/components/GatewaysCard.tsx

packages/ui/src/Wrapper.tsx Show resolved Hide resolved
packages/ui/src/CopyInput.tsx Outdated Show resolved Hide resolved
apps/guardian-ui/src/languages/en.json Show resolved Hide resolved
const { t } = useTranslation();
const { api } = useAdminContext();
const [versions, setVersions] = useState<Versions>();
const [epochCount, setEpochCount] = useState<number>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Building on other discussions around a similar topic, wonder when we need to migrate this to bigint.

const [gateways, setGateways] = useState<Gateway[]>([]);

const lnModuleId = config
? Object.entries(config.client_config.modules).find(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder about some utils around modules.

borderTopLeftRadius={0}
borderBottomLeftRadius={0}
size={size}
height={size == 'lg' ? '46px' : '42px'}
height='100%'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be buttonHeight?

Suggested change
height='100%'
height={buttonHeight}

@EthnTuttle
Copy link
Collaborator

Going to approve and merge to unlock all the other PRs.

@EthnTuttle EthnTuttle merged commit f40a263 into master Sep 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants