-
Notifications
You must be signed in to change notification settings - Fork 2
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
initial project creation - getting started form #18
initial project creation - getting started form #18
Conversation
At least the link from the homepage is working and the route to the new_project page is working. |
I haven't yet added the translation strings to en.yml or it.yml, but I'm wondering if there's a way to extrapolate the keys automatically from an |
If, for example, you want an arbitrary number of groomsmen, an array is probably a good way to represent that. Rails supports sending form data fields as an array: https://stackoverflow.com/questions/3089849/ruby-on-rails-submitting-an-array-in-a-form If you want to dynamically add or remove fields from the UI, Some light javascript would probably be the way to go, and would be pretty much in line with the Rails way of doing things. Most likely, I think you might want to do both of the above. P.S. I merged the Tailwind PR so you can start using it if you rebase this on master (or merge master into this branch). I think you'll need to run |
so I had to run |
such changes need not be done in this PR branch, but if anything should be done in a separate PR
such changes need not be done in this PR branch, but if anything should be done in a separate PR
I'd say that this is pretty much ready to be pulled in now. It's a start anyways, for an initial form. The submit action doesn't do anything yet, but at least it gives an idea of the initial workflow. Feedback is welcome. |
@@ -13,3 +13,11 @@ | |||
*= require_tree . | |||
*= require_self | |||
*/ | |||
.tangerine { |
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.
We should consider if we want to do this "the tailwind way" by modifying our tailwind.config.js
. Perhaps this could be our display
font, or perhaps we could introduce it as its own font family class. Here's the tailwind docs about fonts.
This doesn't need to prevent this PR from merging, just a thought/suggestion, and we could always do it later.
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.
I have tried defining a tailwind font class in tailwind.config.js
like this:
module.exports = {
theme: {
extend: {
fontFamily: {
'tangerine': ['Tangerine', 'cursive']
}
}
}
}
But it doesn't seem to be working. I apply a font-tangerine
class to the div
that I want styled with the Tangerine Google WebFont, but the font is not applied, and inspecting the browser console, I don't see any rules for a .font-tangerine
class.
Does something need to be run after the tailwind.config.js
file is changed, in order for the changes to be picked up? I tried bin/rails assets:precompile
but it didn't help, I tried npm run postcss:build
but I'm getting an error missing script: postcss:build
.
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.
I wouldn't be using this font too much in the UI in any case, it's more for show on the home page for a nice first impact.
@@ -1,3 +1,23 @@ | |||
<div class="bg-green-300 border-green-600 border-b p-4 m-4 rounded"> | |||
<h1><%= t('hello') %></h1> | |||
<div class="flex flex-wrap overflow-hidden h-screen"> |
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.
This page isn't responsive. That is, it doesn't look good when you make the browser roughly the size of a phone.
If we merge without making the page more responsive, I think we should create an issue to come back and make it more responsive later. At some screen size, it should switch from a horizontal to vertical layout.
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.
for certain, this PR still needs a lot of work, but I figured before I went any further we should check to see if it's going in the right direction for everyone. Further issues can certainly be opened, I'm not a styling expert so any further fixes are quite welcome.
app/views/pages/new_project.html.erb
Outdated
} | ||
``` | ||
--> | ||
<h1 class="py-6 my-6 text-center"><%= t('pages-new_project-title') %></h1> <!-- New wedding booklet project --> |
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.
The string translation comments are fine but also probably unnecessary.
There are usually IDE plugins that can help with this kind of thing, like https://marketplace.visualstudio.com/items?itemName=shanehofstetter.rails-i18n
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.
I tried installing this plugin, but it doesn't seem to be working for me, I'm not sure what's missing to get it working.
<label for="marriage_date" class="block text-sm font-medium text-gray-700 required"> | ||
<%= t('pages-new_project-marriage_date-label') %><!-- The wedding will take place on: --> | ||
</label> | ||
<input type="datetime-local" name="marriage_date" id="marriage_date" class="focus:ring-indigo-500 focus:border-indigo-500 flex-1 block w-full border-gray-300 rounded py-2 px-3 sm:text-sm leading-tight"> |
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.
Could there be a placeholder here? It's unclear what kind of time/date format would be best.
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.
I think pretty much all browsers now have a calendar picker? That should pretty much take care of getting the format right, because it will change between locales anyways and browsers handle that with the calendar picker.
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.
@@ -4,6 +4,10 @@ | |||
root to: 'pages#home' | |||
|
|||
scope "/:locale" do | |||
resources :projects | |||
end | |||
resources :pages do |
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.
This is a bit unconventional. If you'd prefer, we can merge it as-is and create an issue to adjust things in a separate PR.
Typically, our controllers will somewhat mirror our models. That is, if we plan on having a Rails model "project" (which it seems like we would, based on your form), then we'd have a projects_controller.rb
(The C in the "MVC" pattern). There would be a route for /projects/new
, which we'd define with resources :projects, only: [:new]
. This will allow all project-related actions (view, create, update) to be handled within the projects controller with routes like /projects/:id
, /projects/:id/edit
, etc. Rails defines most of that by convention for us.
It's worth noting at this point that rails provides generators to do things like scaffold a controller for us.
See https://guides.rubyonrails.org/routing.html#resource-routing-the-rails-default for info about how routing is typically set up.
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.
I figured there would be some rails generators for stuff like this, but being my first hands-on rails project, I have no idea how the generators work yet. I'm not sure what to expect when running a scaffolding generator, and I still have to understand better what controllers are. So I'm certain this can be done in a better / different way, I just don't have the experience to do it yet. It's an accomplishment for me to have succeeded in getting the button on the home page to go to this form page! So I would say, let's go ahead and merge this, then create an issue and fix it up in a separate PR, because I'm sure someone else will know how to do it better than me right now.
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.
in fact I created a "How tos" section in the wiki to document stuff like this. It would be great to add to that wiki page a practical example of how a generator was used to create a Rails model "project".
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.
this is related to the discussion in issue #5 , so I'd say that after this has been pulled, we can try to break down the sections of the UI to see how they relate to the scaffolding and the user experience workflow. I added a couple more comments on the above issue in these regards.
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.
I just tested using the scaffolding generator on a separate branch, to see what it does and what it looks like.
I ran:
bundle exec rails generate scaffold Project liturgy:integer brideFirstName:string brideLastName:string groomFirstName:string groomLastName:string celebrantNamePrefix:string celebrantFirstName:string celebrantLastName:string church:string city:string weddingdate:datetime
Then I ran:
bundle exec rails db:migrate RAILS_ENV=development
Now when I run the webserver, I can see the basic unstyled form which is automatically created at localhost:3000/en/projects/new
, and I see the links to create
, edit
, update
, back
in the different views. I see how it allows to create a new project which will then be listed at localhost:3000/en/projects/
, and will be accessible at localhost:3000/en/projects/1
.
Is there any way to use a unique hash instead of a progressive integer, to identify resources that are created?
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.
It seems this is the way to use a hash: https://pawelurbanek.com/uuid-order-rails
Perhaps not ready to merge just yet, but here's an initial idea for a "Get Started" workflow. A "Get Started" button on the homepage should take you to an initial project creation form, where you can start filling out the information needed to get the project going.
The whole wedding party area of the form needs to be completed, but in order to be able to dynamically add new elements to the form (for a new wedding party member, with all related information), should this kind of dynamic interaction be done with javascript? Or is there a "rails" way of doing this?