Feature: Hide live preview input and button #3968
Conversation
KevinMulhern
added
the
Type: Enhancement
KevinMulhern
changed the title
KevinMulhern
changed the title
| @@ -8,7 +8,10 @@ | |||
|
|
|||
| <div class="flex flex-col md:flex-row md:items-center"> | |||
| <%= link_to 'View code', project_submission.repo_url, target: '_blank', rel: 'noreferrer', class: 'button button--gray font-semibold md:mr-4', data: { test_id: 'view-code-btn' } %> | |||
| <%= link_to 'Live preview', project_submission.live_preview_url, target: '_blank', rel: 'noreferrer', class: 'button button--gray font-semibold mt-5 md:mt-0 md:mr-4', data: { test_id: 'live-preview-btn' } %> | |||
|
|
|||
| <% if project_submission.lesson.has_live_preview? %> | |||
There was a problem hiding this comment.
Mega nit: Demeter
A delegated previewable? or similar method might be a nice minor addition to the project submission model? I was going to suggest doing it in the component, but we're calling the same logic from the form too
There was a problem hiding this comment.
Nice! previewable is a much better name. What do you think about us renaming the db column to that? we'd get those nice predicate methods for free.
There was a problem hiding this comment.
Yeah mint even better, love that ❤️
There was a problem hiding this comment.
I created a follow up issue to rename it #4003
Best to do it after the react version gets the 🪓
| it 'adds the submission without a preview' do | ||
| Pages::ProjectSubmissions::Form | ||
| .new(has_live_preview: false) | ||
| .open | ||
| .fill_in | ||
| .submit | ||
|
|
||
| within(:test_id, 'submissions-list') do | ||
| expect(page).to have_content(user.username) | ||
| expect(page).to have_link('View code') | ||
| expect(page).not_to have_link('Live preview') |
There was a problem hiding this comment.
I haven't done thing kind of thing much before, but it intrigues me. Do you find it generally helpful? Is there a risk of drifting away from reality, since there's such a test specific interface? Or is it more about convenience in this case?
Curious to hear your thoughts in general on the goals / benefits / downsides of an approach like this
There was a problem hiding this comment.
Sorry @ChargrilledChook, I'm probably being very dense lol. But is this about the page object or the line you commented on expect(page).not_to have_link('Live preview')?
There was a problem hiding this comment.
No worries and my bad, re-reading my comment it is a bit cryptic / stream of consciousness 😅
You're spot on though, I'm referring to the page object Pages::ProjectSubmissions::Form
There was a problem hiding this comment.
No worries @ChargrilledChook , happy to explain the approach
I've been using them for a few years at work / on side projects. Heavily inspired by Thoughtbot / Evil Martians and hating having to spend a lot of time figuring out what large specs were actually doing lol.
Two biggest benefits for me:
- Readability - As time goes on and they grow, Capyabara tests tend get harder to read. Mostly because they're imperative with lots of details on how something is done, think of all the
click_button,click_linkandfill_inmethods calls. Page objects allow for far more expressive and declarative tests - "what will be done" kind of deal. A bunch of related steps can be encapsulated and called with one easy to grok method call - likeform.fill_in_and_submit. - Reusability - Once you have a page object, it makes all future related tests easier and quicker to write. Theres a more tangible benefit with them on larger systems where there will be a few core forms/pages which many different system tests need to interact with. For us, it's been great for the project submissions where we need different tests for editing, flagging, voting etc.
Is there a risk of drifting away from reality, since there's such a test specific interface? Or is it more about convenience in this case?
I haven't had too many issues with things drifting. But definitely see how they could. I think the trick is not to overuse them. If everything is a page object, you'd be jumping from file to file and absolutely hating the test suite.
Forms are always great candidates for page objects, they need the same steps, potentially in many specs and their interface is always pretty standard, they all need fill_in and submit methods.
These articles were a big help when I first started using them:
https://thoughtbot.com/blog/better-acceptance-tests-with-page-objects
https://evilmartians.com/chronicles/system-of-a-test-2-robust-rails-browser-testing-with-siteprism
Because: * Some solutions do not accept previews - for example the early Ruby projects. This commit: * If the project does not accept live previews... * Hide the live preview input. * hide the live preview button
KevinMulhern
force-pushed
the
chore/hide-preview-input
branch
from
f530c60
to
2cc345c
Compare
Because: * Some solutions do not accept previews - for example the early Ruby projects. This commit: * If the project does not accept live previews... * Hide the live preview input. * hide the live preview button
Because:
This commit: