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

Convert submenu to Vue #4739

Merged
merged 9 commits into from
Jul 26, 2018
Merged

Convert submenu to Vue #4739

merged 9 commits into from
Jul 26, 2018

Conversation

sharkykh
Copy link
Contributor

@sharkykh sharkykh commented Jul 24, 2018

This is set for v0.2.7 because it fixes the bug with the confirmation dialogs for removing shows, clearing history, etc.

To do:

  • Remove the check for controller !== 'schedule' && action !== 'previewRename'.
    For previewRename - either remove the submenu argument from the handler,
    or keep it - if we want it. Make sure removing that submenu has no effect.
  • Show selector - check location.pathname has displayShow?
  • Show selector - get current show slug from the url?
  • Cherry-picked from Convert titles and headers to VueRouter #4663 - VueRouter: Disable not-found redirection for now

@sharkykh sharkykh added this to the 0.2.7 milestone Jul 24, 2018
@OmgImAlexis OmgImAlexis mentioned this pull request Jul 24, 2018
@sharkykh
Copy link
Contributor Author

This is as far as I can go with this at the moment. The last Python conversion requires that all the routes be listed on the VueRouter (#4663).
This is good enough for now, though.

@@ -61,7 +61,7 @@

<app-header></app-header>
% if submenu:
<%include file="/partials/submenu.mako"/>
<submenu></submenu>
Copy link
Collaborator

@OmgImAlexis OmgImAlexis Jul 25, 2018

Choose a reason for hiding this comment

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

All components should use a 2 section name like sub-menu, app-header, etc. to avoid mixing with custom-html elements. I think the @vue/recommended eslint rule adds this. We currently use the basic version.

<template v-for="menuItem in submenu">
<app-link :key="menuItem.title" :href="menuItem.path" class="btn-medusa top-5 bottom-5"
v-if="!menuItem.confirm">
<span :class="`pull-left ${menuItem.icon || ''}`"></span> {{ menuItem.title }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use :class="['pull-left', menuItem.icon]" instead.

OmgImAlexis
OmgImAlexis previously approved these changes Jul 25, 2018
Copy link
Collaborator

@OmgImAlexis OmgImAlexis left a comment

Choose a reason for hiding this comment

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

LGTM

p0psicles
p0psicles previously approved these changes Jul 25, 2018
OmgImAlexis
OmgImAlexis previously approved these changes Jul 25, 2018
Copy link
Collaborator

@OmgImAlexis OmgImAlexis left a comment

Choose a reason for hiding this comment

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

LGTM

@sharkykh sharkykh dismissed stale reviews from OmgImAlexis and p0psicles via f6e1ddc July 25, 2018 14:02
@OmgImAlexis OmgImAlexis merged commit 6e9cfd2 into develop Jul 26, 2018
@OmgImAlexis OmgImAlexis deleted the vueify/submenu branch July 26, 2018 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants