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/limel menu cascading items #2473

Closed
wants to merge 2 commits into from

Conversation

TommyLindh2
Copy link
Contributor

@TommyLindh2 TommyLindh2 commented Sep 11, 2023

fix: https://github.com/Lundalogik/solution-helpdesk-poc/issues/23

Review:

  • Commits are atomic
  • Commits have the correct type for the changes made
  • Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

  • Chrome
  • Edge
  • Firefox

Linux:

  • Chrome
  • Firefox

macOS:

  • Chrome
  • Firefox
  • Safari

Mobile:

  • Chrome on Android
  • iOS

@TommyLindh2 TommyLindh2 marked this pull request as draft September 11, 2023 13:59
@github-actions
Copy link

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-2473/

@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch 7 times, most recently from 2b3c8d8 to e704f63 Compare September 14, 2023 08:31
@TommyLindh2 TommyLindh2 changed the base branch from next to create-breadcrumbs-component September 14, 2023 09:02
@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch from e704f63 to 17fb3b1 Compare September 14, 2023 10:50
@BjornOstberg BjornOstberg force-pushed the feat/limel-menu-cascading-items branch from 17fb3b1 to b88c763 Compare September 14, 2023 13:10
@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch 4 times, most recently from 25af00d to cb40e0b Compare September 18, 2023 11:58
@TommyLindh2 TommyLindh2 force-pushed the create-breadcrumbs-component branch from cd44fa4 to 62561be Compare September 18, 2023 11:59
@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch 12 times, most recently from c310b18 to 2ab5a2a Compare September 18, 2023 14:49
@Kiarokh Kiarokh force-pushed the create-breadcrumbs-component branch from 2607213 to da7fe3f Compare September 19, 2023 10:34
@BjornOstberg BjornOstberg force-pushed the feat/limel-menu-cascading-items branch 5 times, most recently from 7f72b0b to c8ca354 Compare October 6, 2023 08:43
src/components/menu/menu.types.ts Outdated Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.types.ts Show resolved Hide resolved
@BjornOstberg BjornOstberg force-pushed the feat/limel-menu-cascading-items branch 3 times, most recently from 453d11b to 5251e58 Compare October 13, 2023 08:53
Copy link
Contributor

@jgroth jgroth left a comment

Choose a reason for hiding this comment

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

Not sure why there are spaces in some random places 😄

src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.types.ts Outdated Show resolved Hide resolved
src/components/menu/menu.types.ts Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
src/components/menu/menu.tsx Outdated Show resolved Hide resolved
@BjornOstberg BjornOstberg force-pushed the feat/limel-menu-cascading-items branch 2 times, most recently from a285990 to 1048174 Compare October 23, 2023 12:26
@Kiarokh
Copy link
Contributor

Kiarokh commented Oct 31, 2023

There seems to be a few comments from Johan that are not answered yet.

After fixing them, I’d suggest that you squash all the fixup commits, and then split the PR into to few meaningful PRs that can be reviewed and merged separately. This makes the work less scary for the reviewers.

For example, you can have one PR for enabling search, with a couple of examples.
Next PR could be for enabling lazy loading + examples.
Another PR can be about adding nested menus.

Which one you put up first for review is up to you. You know better how things are connected. Just keep in mind that the repo should be able to run or be deployed free of bugs or problems, after each commit that you add.

@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch 5 times, most recently from 61dddd3 to 1ff48ec Compare October 31, 2023 14:09
@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch from 1ff48ec to c75f289 Compare October 31, 2023 14:54
@TommyLindh2 TommyLindh2 force-pushed the feat/limel-menu-cascading-items branch from c75f289 to bc222b9 Compare October 31, 2023 14:55
@TommyLindh2 TommyLindh2 removed the request for review from adrianschmidt October 31, 2023 15:07
@TommyLindh2
Copy link
Contributor Author

This PR have been through a lot and it looks like it's huge when looking at the comments.
That was a review from the public API/interfaces of the branch, So I will close this and create a separate one for the actual code.

@TommyLindh2 TommyLindh2 mentioned this pull request Oct 31, 2023
13 tasks
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.

5 participants