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

ui: Public Collections UI Nitpicks #2287

Merged
merged 10 commits into from
Jan 9, 2025

Conversation

Shrinks99
Copy link
Member

@Shrinks99 Shrinks99 commented Jan 8, 2025

Fixes #2283

Public collections are looking great! This PR covers the few little things I found when exploring the feature.

Changes

  • Removes share link from the dialogue footer
    • I would rather keep that space exclusively reserved for dialogue buttons, moved below the visibility selector so that it adds an extra indicator of what can and cannot be done when a collection is not publicly visible.
  • Removes stickied collection navigation, replaces with improved viewport-based scaling!
  • Adds a max-width for the collection description in the logged in view.
    • No more full page width text!
  • Moves the markdown editor buttons to below the editor
    • Controls are now In-line with how we handle dialogue options elsewhere, fixes a minor responsive design issue.
  • Minor copy changes

Screenshots

Moved share link (not visible because it's private)
Screenshot 2025-01-07 at 11 10 52 PM

Moved share link (visible)
Screenshot 2025-01-07 at 11 10 13 PM

Copy updates

Screenshot 2025-01-07 at 11 38 30 PM

Line length change

Screenshot 2025-01-07 at 11 35 06 PM

Composer Button Relocation

Screenshot 2025-01-07 at 11 35 16 PM

Caveats

@Shrinks99 Shrinks99 requested review from SuaYoo and emma-sg January 8, 2025 04:36
maxlength=${4000}
></btrix-markdown-editor>
<div class="flex justify-center leading-relaxed">
<div class="w-full md:max-w-[783px]">
Copy link
Member

@SuaYoo SuaYoo Jan 8, 2025

Choose a reason for hiding this comment

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

Where does the 783px value come from? Can we use one of the CSS variables here, or em or ch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Comes from being sloppy while trying to figure out a comfortable value 🙃

Do we have a global value that we use for max-width with text? On the site this is part of the prose tailwind typography setting (which I would rather set separately because it's somewhat regularly overruled)... Maybe we should be using that here too?

</div>
return html` <section class="overflow-hidden rounded-lg border">
<replay-web-page
class="h-[calc(100vh-6.5rem)]"
Copy link
Member Author

@Shrinks99 Shrinks99 Jan 8, 2025

Choose a reason for hiding this comment

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

I'll also note that while I consider this a reasonably good solution (and probably better than using aspect ratio?), it is somewhat fragile and going forward if we make layout changes we'll need to remember to update it. Open to better ideas!

@emma-sg
Copy link
Member

emma-sg commented Jan 8, 2025

A couple more nitpicks — I'll address!


Hover issues:

Screen.Recording.2025-01-08.at.3.27.02.PM.mov

Checkmark is hard to see on mostly white image:

Screenshot 2025-01-08 at 3 24 33 PM

@emma-sg
Copy link
Member

emma-sg commented Jan 8, 2025

Screenshot 2025-01-08 at 4 59 37 PM Updated check mark!

@SuaYoo SuaYoo force-pushed the public-collections-ui-improvements branch from bb6ae25 to c6b39ff Compare January 8, 2025 23:47
@SuaYoo SuaYoo self-requested a review January 8, 2025 23:57
Shrinks99 and others added 10 commits January 8, 2025 15:58
- "Default" is generally not a good descriptor, could be anything!
  - That said, we also show the outcome in the image on the left so by no means a serious UI sin as is :)
- Helps ensure the window is always visible in the viewport
- Calc accommodates for the navigation & footer height on desktop
- Removes sticky and white shadow blur
@SuaYoo SuaYoo force-pushed the public-collections-ui-improvements branch from 0a9de57 to 518f64d Compare January 8, 2025 23:58
Copy link
Member

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

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

Looks great!

@SuaYoo SuaYoo merged commit 5db234f into public-collections-feature Jan 9, 2025
7 checks passed
@SuaYoo SuaYoo deleted the public-collections-ui-improvements branch January 9, 2025 00:00
SuaYoo added a commit that referenced this pull request Jan 9, 2025
- Removes share link from the dialogue footer
- Removes stickied collection navigation, replaces with improved
viewport-based scaling!
- Adds a max-width for the collection description in the logged in view.
- Moves the markdown editor buttons to below the editor
- Controls are now In-line with how we handle dialogue options
elsewhere, fixes a minor responsive design issue.
- Minor copy changes

---------

Co-authored-by: emma <[email protected]>
Co-authored-by: sua yoo <[email protected]>
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.

3 participants