Feat/UI ux enhancement framegear #1241

adarshswaminath · 2024-09-12T10:16:21Z

Enhance Home Component with Layout, Loading State, and Placeholder Improvements

What changed? Why?

This PR includes the following changes:

Improved Responsiveness and Layout:
- Updated the grid layout in the Home component to ensure better responsiveness: now a single column on smaller screens and two columns on larger screens.
- Added padding (p-2) to the main container for better spacing and improved layout on all screen sizes.
Loading State in FrameInput Component:
- Added a loading state to the fetchFrame API call in FrameInput. The button now reflects "Loading..." when the API call is in progress, enhancing user experience.
Placeholder for Missing Images:
- Added placeholders to handle missing images, ensuring the UI remains clean and informative even when images are unavailable.

Notes to reviewers

Please focus on checking the responsiveness of the layout across different screen sizes.
Verify that the loading state in the FrameInput component works as expected.
Check if the placeholders for missing images are working correctly.

How has it been tested?

Manual testing was performed to ensure the layout is responsive on both small and large screens.
The loading state was tested by simulating a slow API call, confirming the button updates appropriately.
Missing images were tested by intentionally breaking image links, verifying the placeholders display properly.

Screenshots:

…e handling

vercel · 2024-09-12T10:16:26Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
onchainkit-coverage	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 13, 2024 3:33am
onchainkit-routes	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 13, 2024 3:33am

vercel · 2024-09-12T10:16:28Z

@adarshswaminath is attempting to deploy a commit to the Coinbase Team on Vercel.

A member of the Team first needs to authorize it.

Zizzamia · 2024-09-12T11:57:31Z

framegear/components/Frame/Frame.tsx

-        className={`w-full rounded-t-xl ${imageAspectRatioClassname} object-cover`}
-        src={image.src}
+        className={`w-full rounded-t-xl ${imageAspectRatioClassname} ${image.src ? "object-cover" : "object-contain"}`}
+        src={image.src ? image.src : "https://demofree.sirv.com/nope-not-here.jpg?w=750"}


Sorry we can not have links here on random websites.

would it be acceptable if I add the images directly to the public folder of the project and reference them from there?

yup, but only if it's a cool image. If it's not, is probably best have something else as default.

I think is best we avoid add those kind of images so far. As we want our designer to take a look at it.

Ok, I'll update the image based on the designer's review.

Zizzamia · 2024-09-12T11:58:41Z

framegear/components/FrameInput/FrameInput.tsx

@@ -28,12 +31,12 @@ export function FrameInput() {
        />
      </label>
      <button
-        className="h-[40px] self-end rounded-full bg-white text-black"
+        className={`h-[40px] self-end rounded-full ${isLoading ? "bg-neutral-800 text-white" : "bg-white text-black"}`}


I believe you can use clx on Framegear as well.

Okay, I will implement the clx

Zizzamia · 2024-09-12T11:59:21Z

I am going to have @brendan-defi review this as well, as he is leading our effort to keep polish Framegear.

brendan-defi · 2024-09-13T02:15:23Z

framegear/components/Frame/Frame.tsx

@@ -76,10 +85,6 @@ function ValidFrame({ metadata }: { metadata: FrameMetadataWithImageObject }) {
 }

 function ErrorFrame() {
-  // TODO: implement -- decide how to handle


Definitely acknowledge that this isn't elegant, but let's not remove. We will work on cleaning up these comments.

brendan-defi · 2024-09-13T02:15:37Z

framegear/components/Frame/Frame.tsx

@@ -116,14 +121,12 @@ function FrameButton({

  const handleClick = useCallback(async () => {
    if (button?.action === 'post' || button?.action === 'post_redirect') {
-      // TODO: collect user options (follow, like, etc.) and include


as above, let's keep this comment

brendan-defi · 2024-09-13T02:15:43Z

framegear/components/Frame/Frame.tsx

      const confirmAction = async () => {
        const result = await postFrame(
          {
            buttonIndex: index,
            url: (button as any).postUrl!,
            state: JSON.stringify(state),
-            // TODO: make these user-input-driven


brendan-defi · 2024-09-13T02:15:59Z

framegear/components/Frame/Frame.tsx

@@ -136,7 +139,6 @@ function FrameButton({
          },
          mockFrameOptions
        );
-        // TODO: handle when result is not defined


brendan-defi · 2024-09-13T02:16:05Z

framegear/components/Frame/Frame.tsx

@@ -162,7 +164,6 @@ You can test this action on the official Warpcast validator: https://warpcast.co
 (must deploy frame to a publicly accessible URL)`
      );
    }
-    // TODO: implement other actions (mint, etc.)


brendan-defi · 2024-09-13T02:17:16Z

I am going to have @brendan-defi review this as well, as he is leading our effort to keep polish Framegear.

Thanks @Zizzamia !

@adarshswaminath Thanks for taking the time to work on this. The responsiveness and the loading state on the fetch-button are nice, good work. :-)

Left a few comments. Most importantly, I don't think we should add an placeholder image like this though. This generic 404 isn't really the right message (the issue isn't necessarily a 404, it's that the image for that frame isn't valid, which can happen for many reasons). With these changes I think you'll be g2g.

Thanks!

…valid image frames

adarshswaminath · 2024-09-13T03:32:10Z

@brendan-defi Thankyou for the review :) I don’t know why I removed those comments earlier :(
I have now added them back. Instead of the placeholder image, I have added a 'No Image Found' message, which is straightforward for the user to understand that there is no image to display.

brendan-defi · 2024-09-13T16:01:02Z

@brendan-defi Thankyou for the review :) I don’t know why I removed those comments earlier :( I have now added them back. Instead of the placeholder image, I have added a 'No Image Found' message, which is straightforward for the user to understand that there is no image to display.

Thanks @adarshswaminath, nice work :-)

adarshswaminath · 2024-09-13T16:43:04Z

Thank you! Actually, these comments from @brendan-defi and @Zizzamia are really exciting for a newbie like me. Getting positive feedback from such a great team feels like the next level :-) . Is this PR ready to merge?

adarshswaminath added 3 commits September 12, 2024 15:16

feat:Improve UX with placeholder for missing images and enhanced imag…

c3a3377

…e handling

feat(ui): add loading state for frame fetching in FrameInput component

6141e95

feat(ui): improve Home component responsiveness and layout

73de46b

vercel bot deployed to Preview – onchainkit-routes September 12, 2024 10:16 View deployment

vercel bot deployed to Preview – onchainkit-coverage September 12, 2024 10:18 View deployment

Zizzamia reviewed Sep 12, 2024

View reviewed changes

adarshswaminath added 2 commits September 12, 2024 19:21

refactor: integrate clsx for conditional classnames in Frame component

6ba350f

feat: update placeholder image to local file when image.src is missing

046e8b7

vercel bot deployed to Preview – onchainkit-routes September 12, 2024 15:03 View deployment

vercel bot deployed to Preview – onchainkit-coverage September 12, 2024 15:04 View deployment

brendan-defi reviewed Sep 13, 2024

View reviewed changes

adarshswaminath added 2 commits September 13, 2024 08:29

chore: add temporary comments from previous code, to be cleaned up later

1393120

fix(ui): replace 404 placeholder with "No Image Found" message for in…

fa98c86

…valid image frames

vercel bot deployed to Preview – onchainkit-routes September 13, 2024 03:31 View deployment

vercel bot deployed to Preview – onchainkit-coverage September 13, 2024 03:33 View deployment

brendan-defi approved these changes Sep 13, 2024

View reviewed changes

Zizzamia approved these changes Sep 14, 2024

View reviewed changes

Zizzamia merged commit 76c1edb into coinbase:main Sep 14, 2024
10 of 13 checks passed

github-actions bot mentioned this pull request Sep 16, 2024

chore: version packages v0.32.0 🌊 #1259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/UI ux enhancement framegear #1241

Feat/UI ux enhancement framegear #1241

adarshswaminath commented Sep 12, 2024

vercel bot commented Sep 12, 2024 •

edited

Loading

vercel bot commented Sep 12, 2024

Zizzamia Sep 12, 2024

adarshswaminath Sep 12, 2024

Zizzamia Sep 12, 2024

adarshswaminath Sep 12, 2024

Zizzamia Sep 13, 2024

adarshswaminath Sep 13, 2024

Zizzamia Sep 12, 2024

adarshswaminath Sep 12, 2024

Zizzamia commented Sep 12, 2024

brendan-defi Sep 13, 2024

brendan-defi Sep 13, 2024

brendan-defi Sep 13, 2024

brendan-defi Sep 13, 2024

brendan-defi Sep 13, 2024

brendan-defi commented Sep 13, 2024

adarshswaminath commented Sep 13, 2024

brendan-defi commented Sep 13, 2024

adarshswaminath commented Sep 13, 2024

Feat/UI ux enhancement framegear #1241

Feat/UI ux enhancement framegear #1241

Conversation

adarshswaminath commented Sep 12, 2024

Enhance Home Component with Layout, Loading State, and Placeholder Improvements

What changed? Why?

Notes to reviewers

How has it been tested?

Screenshots:

vercel bot commented Sep 12, 2024 • edited Loading

vercel bot commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zizzamia commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendan-defi commented Sep 13, 2024

adarshswaminath commented Sep 13, 2024

brendan-defi commented Sep 13, 2024

adarshswaminath commented Sep 13, 2024

vercel bot commented Sep 12, 2024 •

edited

Loading