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

feat: schedule list item action menu #230

Merged
merged 17 commits into from
Oct 5, 2024

Conversation

caseycharleston
Copy link
Contributor

@caseycharleston caseycharleston commented Sep 28, 2024

feat: floating menu for vertical dot popup for schedule list item component using @headlessui-float/react
feat: Schedule name will remain the same upon a rename if the schedule has a duplicate name of an existing schedule and it is renamed to the base name of the original schedule (ex. Schedule (2) is renamed to Schedule, and a schedule named Schedule already exists. It will stay as Schedule (2) instead of incrementing to 3).
feat: dialog popups added for deleting schedules and letting the user know deleting the active schedule is not allowed.

refactor: renaming a schedule an already existing name will be handled via "duplicate schedule logic"
refactor: creating a schedule will also be handled with "duplicate schedule logic" when making multiple new schedules at once

Demo:

Sept.28.Screen.Recording.2.mp4
Sept.28.Screen.Recording.mp4

This change is Reviewable


This change is Reviewable

Copy link
Member

@IsaDavRod IsaDavRod left a comment

Choose a reason for hiding this comment

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

Renaming a schedule in the extension popup does not reflect in the Calendar view schedule list unless you refresh the page. (see photo below)

image

src/pages/background/lib/duplicateSchedule.ts Outdated Show resolved Hide resolved
src/pages/background/lib/renameSchedule.ts Outdated Show resolved Hide resolved
@Razboy20 Razboy20 added awaiting-changes feature UI/UX-figma PRs that fulfill a task on the UI/UX & Feature Roadmap labels Sep 29, 2024
@Razboy20
Copy link
Member

Renaming a schedule in the extension popup does not reflect in the Calendar view schedule list unless you refresh the page. (see photo below)

image

This all falls within the component issue unfortunately. Not applicable to this PR.

Copy link
Member

@Razboy20 Razboy20 left a comment

Choose a reason for hiding this comment

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

Looks good overall! I'm surprised Isaiah didn't mention it, but let's line up the design style of the prompts to match those on the Figma (buttons, etc.).

Also, I would argue to remove the background on the triple dot menu icon.

Copy link
Member

@IsaDavRod IsaDavRod left a comment

Choose a reason for hiding this comment

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

Menu works great! It is missing Figma styles for the border, padding, text and colors. Please fix the UI part and the PR should be good to go! Link to Figma component is here

CURRENT:
image

FIGMA:
image

@DereC4
Copy link
Member

DereC4 commented Oct 2, 2024

image

hokly hawk tuah

Copy link
Member

@IsaDavRod IsaDavRod left a comment

Choose a reason for hiding this comment

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

needs a few more UI tweaks!
View the Figma Prototype (presenter):

  • When hovering over an item, the hover fill should have rounded corners and should not touch the menu's borders. (idk how much of a hassle this would be tho)
  • The menu's borders should be Secondary Colors/Off-white (just like the confirmation popup dialog's border when deleting a schedule)

CURRENT:
image

FIGMA:
image

Copy link
Member

@IsaDavRod IsaDavRod left a comment

Choose a reason for hiding this comment

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

LGTM yayy

@IsaDavRod
Copy link
Member

I would argue to remove the background on the triple dot menu icon.

If u can Casey, remove the background on the triple dot menu icon like Elie suggested

@IsaDavRod IsaDavRod dismissed Razboy20’s stale review October 4, 2024 23:56

UI issues fixed

@Razboy20 Razboy20 self-requested a review October 5, 2024 04:08
@Razboy20 Razboy20 requested a review from doprz October 5, 2024 04:08
Copy link
Collaborator

@doprz doprz left a comment

Choose a reason for hiding this comment

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

LGTM, great job with this PR!

@doprz doprz merged commit 15fc369 into Longhorn-Developers:main Oct 5, 2024
6 checks passed
@caseycharleston caseycharleston deleted the options-popup branch October 8, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-changes feature UI/UX-figma PRs that fulfill a task on the UI/UX & Feature Roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants