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

MMT-1151 display banner if open draft exists for variable #154

Merged
merged 2 commits into from
Sep 29, 2017
Merged

Conversation

macrouch
Copy link
Contributor

No description provided.


it 'displays the draft' do
within '.eui-breadcrumbs' do
expect(page).to have_content('Drafts')
Copy link
Collaborator

Choose a reason for hiding this comment

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

not totally necessary, but you could make this more specific and check for 'Variable Drafts'


it 'displays the draft' do
within '.eui-breadcrumbs' do
expect(page).to have_content('Drafts')
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment as above

<%= link_to 'Yes', collection_draft_path(options[:draft_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-view-draft-link' %>
<%= link_to 'Yes', edit_collection_draft_path(options[:draft_id], form: options[:draft_form]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-edit-draft-link' %>
<% if options[:variable] %>
<%= link_to 'Yes', variable_draft_path(options[:draft_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-view-draft-link' %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ids for the links to view and edit the variable drafts are identical to the ids for links for the collection drafts here. They should be unique, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only one set of IDs is rendered onto the page, so they are unique in the browser

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, ok.

@macrouch macrouch merged commit 828de40 into master Sep 29, 2017
@macrouch macrouch deleted the MMT-1151 branch October 4, 2023 19:27
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.

3 participants