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

Add panel type mini-icons #562

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gordlin
Copy link
Member

@gordlin gordlin commented Feb 25, 2025

Related Item(s)

Supports (NOT donethanks'd) #447

Changes

  • [FEATURE] Adds panel type mini-icons to the ToC, displaying all the panels that a slide config has.
    • The icon indicates the type of panel.
    • Upon hover, a tooltip appears that displays:
      • The panel number
      • The panel type
      • The title of the panel, if available, truncated to at most 130 characters (can easily be modified if anyone has feedback)
  • [FEATURE] Display the type of panel in panel selection buttons (Left Panel/Right Panel/Fullscreen Panel) (e.g. Text for text panels).

Notes

Text panel mini-icon, and new panel selection buttons
image

Really long title
image

Nice try
image

Testing

Steps:

  1. Open any product.
  2. In the table of contents, notice that each config has mini-icons indicating the panels it has.
  3. Try playing with the slides (e.g. update panel types, change panel titles, duplicating slides, etc.) The mini-icons should seamlessly update.
  4. Open any editor. Notice that the buttons to change panels (Left Panel/Right Panel/Fullscreen Panel) display the type of panel they currently contain (e.g. Text for text panels).
  5. Try changing a panel's type. The corresponding panel button should change its text to the new type.

This change is Reviewable

@gordlin gordlin added the PR: Frontend PR that primarily involves frontend changes. UI experts and CSS Wizards are asked to review. label Feb 25, 2025
Copy link

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/shared-map-config

Copy link
Member

@IshavSohal IshavSohal left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion


a discussion (no related file):
Changes are looking good to me.

Noticed a fairly minor issue. When the icon for a slideshow panel is hovered, two tooltips appear: one indicating the panel's info, and one that says carousel-slide. Likewise for map panels, there is an additional tooltip that says pin-location. I'm assuming these are the names of the svg's. Not sure why it only shows up for these two though.

Copy link
Member

@spencerwahl spencerwahl left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion (waiting on @IshavSohal)


a discussion (no related file):

Previously, IshavSohal (Ishav Sohal) wrote…

Changes are looking good to me.

Noticed a fairly minor issue. When the icon for a slideshow panel is hovered, two tooltips appear: one indicating the panel's info, and one that says carousel-slide. Likewise for map panels, there is an additional tooltip that says pin-location. I'm assuming these are the names of the svg's. Not sure why it only shows up for these two though.

there are title elements within those icons, removing those should fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Frontend PR that primarily involves frontend changes. UI experts and CSS Wizards are asked to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants