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 menu item description #76

Closed
wants to merge 1 commit into from
Closed

Conversation

anoblet
Copy link
Collaborator

@anoblet anoblet commented Jan 14, 2021

No description provided.

@anoblet anoblet changed the title Dev add menu item description Jan 14, 2021
@lkraav
Copy link

lkraav commented Feb 9, 2021

@anoblet it seems cxl-app-layout work has been pushed into this PR? I'd like to have another look at the menu item description solution and see if I can merge it.

Can you rebase to latest and re-push?

@anoblet
Copy link
Collaborator Author

anoblet commented Feb 9, 2021

@lkraav Fixed!

@anoblet anoblet requested a review from lkraav February 9, 2021 13:59
@lkraav
Copy link

lkraav commented Feb 9, 2021

Fixed!

Thanks, but are there a bunch of eslint auto-fix changes in here, or is your code editor doing something? Can you attempt a clean "relevant changes only" commit?

@anoblet
Copy link
Collaborator Author

anoblet commented Feb 9, 2021

The es-lint auto fixes are part of the pre-commit hook. Apart from editing the file in Github, I'm not sure how to disable them.

@lkraav
Copy link

lkraav commented Feb 9, 2021

The es-lint auto fixes are part of the pre-commit hook. Apart from editing the file in Github, I'm not sure how to disable them.

I find it curious that running yarn lint does not produce the same changes 🤔

You can commit without linters using --no-verify.

@lkraav
Copy link

lkraav commented Feb 9, 2021

I find it curious that running yarn lint does not produce the same changes thinking
You can commit without linters using --no-verify.

It's prettier making these changes, but it isn't part of lint script.

I'll see if I can run it on codebase manually so we can get the baseline fixed.

@lkraav
Copy link

lkraav commented Feb 9, 2021

OK rebase on latest master, let's see what that looks like.

@anoblet
Copy link
Collaborator Author

anoblet commented Feb 9, 2021

Looks a lot cleaner now.

@lkraav
Copy link

lkraav commented Feb 9, 2021

Hmm, in latest FF, I get an exception and menu loading crashes

18:32:07.213 Uncaught (in promise) TypeError: Invalid attempt to spread non-iterable instance.
In order to be iterable, non-array objects must have a [Symbol.iterator]() method.
    _nonIterableSpread main.d5be0d923e9eb240271c.bundle.js:2596
    _toConsumableArray main.d5be0d923e9eb240271c.bundle.js:2592
    _updatedContextMenuItems cxl-marketing-nav.js:281
    _updatedContextMenuItems cxl-marketing-nav.js:268
    _updatedContextMenuItems cxl-marketing-nav.js:267
    updated cxl-marketing-nav.js:201
    performUpdate updating-element.ts:791
    _callee$ updating-element.ts:724
    tryCatch runtime.js:63
    invoke runtime.js:293
    defineIteratorMethods runtime.js:118
    asyncGeneratorStep vendors~main.d5be0d923e9eb240271c.bundle.js:63771
    _next vendors~main.d5be0d923e9eb240271c.bundle.js:63793
main.d5be0d923e9eb240271c.bundle.js:2596:9

@anoblet
Copy link
Collaborator Author

anoblet commented Feb 9, 2021

Fixed

@kylebrodeur
Copy link

@saas786 saas786 closed this Sep 6, 2022
@saas786
Copy link

saas786 commented Sep 7, 2022

closed in favour of #220

@saas786 saas786 removed the request for review from lkraav September 7, 2022 06:43
@saas786 saas786 added the enhancement New feature or request label Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants