-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adds read only sidebar/context menu for triggering edit #107
Conversation
luketlancaster
commented
Oct 23, 2019
- Add the Clubhouse Story ID: [ch21123]
@@ -48,7 +48,7 @@ const LayoutView = View.extend({ | |||
</div> | |||
</div> | |||
<div class="flex-region list-page__list"> | |||
<table><tr> | |||
<table class="w-100 js-list-header"><tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need the fixWidth
stuff that the patient list has? Now that it has this header row?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is definitely doesn't need the .js-
hook unless it's used in the js. I'm surprised it needs the w-100
I thought table were 100% by default.. maybe it's a firefox thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep so w-100
👍 js-list-header
👎
class: 'program-sidebar__edit', | ||
text: i18n.menuOptions.edit, | ||
}, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left this more complex in the event that we do the delete at some point
Codecov Report
@@ Coverage Diff @@
## develop #107 +/- ##
===========================================
+ Coverage 85.55% 97.75% +12.2%
===========================================
Files 94 94
Lines 1592 1603 +11
===========================================
+ Hits 1362 1567 +205
+ Misses 230 36 -194
Continue to review full report at Codecov.
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
const sidebarView = new SidebarView({ model: this.program }); | ||
|
||
this.listenTo(sidebarView, { | ||
edit: this.onEdit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit should be in quotes because it's an event
@@ -48,7 +48,7 @@ const LayoutView = View.extend({ | |||
</div> | |||
</div> | |||
<div class="flex-region list-page__list"> | |||
<table><tr> | |||
<table class="w-100 js-list-header"><tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is definitely doesn't need the .js-
hook unless it's used in the js. I'm surprised it needs the w-100
I thought table were 100% by default.. maybe it's a firefox thing?
<div class="program-sidebar__heading">{{ @intl.admin.sidebar.program.state }}</div> | ||
<div class="program-sidebar__item programs-sidebar__published{{#if published}} is-published{{/if}}"> | ||
{{#if published}}{{fas "toggle-on"}}{{else}}{{far "toggle-off"}}{{/if}} | ||
{{formatMessage (intlGet "admin.list.programsAllViews.itemView.published") published=published}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin.programs.program.sidebar.sidebarLayoutTemplate.published
{{/if}} | ||
<h1 class="program-sidebar__name">{{ name }}</h1> | ||
<div class="program-sidebar__heading">{{ @intl.admin.sidebar.program.description }}</div> | ||
<div class="program-sidebar__item {{#unless details}}is-empty{{/unless}}">{{#if details}}{{ details }}{{else}}{{ @intl.admin.sidebar.program.noDetails }}{{/if}}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin.programs.program.sidebar.sidebarLayoutTemplate.noDetails
1b0224c
to
c302504
Compare
c302504
to
73e3ae1
Compare