senny / corner_stones

capybara building blocks for acceptance tests.
18 stars 8 forks source link

selector to find Link for DeletableRows #20

Open bjoernbur opened 11 years ago

bjoernbur commented 11 years ago

wouldn't it be better to use the existing functionality of rails than using a new css-class to find the link which is used to delete a record?

so you could use:

def attributes_for_row(row)
  super.merge('Delete-Link' => row.first('td *[data-method=delete]'))
end

instead of:

def attributes_for_row(row)
  super.merge('Delete-Link' => row.first('td .delete-action'))
end

so the user wouldn't have to add a css-class which is only used as identifier.

bjoernbur commented 11 years ago

@senny What do you think about this issue? I could write a PR for this...

senny commented 11 years ago

I like the Idea. The code on master did does not look like your snippets anymore it does use the same css-class though:

def delete
  delete_link = node.first('td .delete-action')
  if delete_link
    delete_link.click
  else
    raise "The row '#{attributes}' does not have a delete-link"
  end
end

I think the best solution would be to extract delete_link into it's own method and use the CSS Selector you described above. This would allow the user to overwrite the lookup for the delete_link in special circumstances. We could also think about exposing the selector as an attribute on the table. This could look something like:

table = CornerStones::Table.new('.articles').tap do |t|
        t.extend(CornerStones::Table::SelectableRows)
        t.extend(CornerStones::Table::DeletableRows)
      end
table.delete_link_selector # => 'td *[data-method=delete]'
table.delete_link_selector = 'td .delete-action'

That way one could very simply switch the selector. If the lookup is more complex that a simple selector one could overwrite the #delete_link method.

What do you think?

bjoernbur commented 11 years ago

I prefer the first approach with the delete_link-method... i'll write a pr...