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

update about and business faq #415

Merged
merged 5 commits into from
Jan 18, 2018
Merged

update about and business faq #415

merged 5 commits into from
Jan 18, 2018

Conversation

mi-wood
Copy link
Member

@mi-wood mi-wood commented Jan 7, 2018

Turning #272 into haml

@mi-wood
Copy link
Member Author

mi-wood commented Jan 7, 2018

I think we could go for increasing the paragraph font size on these too. I think this requires creating a new class for the about page

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 9, 2018

Just looking over the code changes, and in vagrant, it all looks pretty good.

  • minor issue: the business FAQ link isn't working in vagrant. I would suggest a relative path such as /business_info rather than https://refugerestrooms.org/business_info but just doing that change didn't fix it on my machine. Might have to set something up in config/routes.rb?

    • Umm, I'm face-palming a bit that I didn't figure this one out quicker...
      • Minor typo: The href to business_info in locales/about.en.yml is just missing an equals sign. e.g.: href='so-and-so'
    • I personally believe a relative URL is better so we can test these things in vagrant more easily.
    • applies to link to about page from business_info page as well
  • Just pointing out: this would probably make a merge conflict for Spanish translation  #413, so we should probably merge that first for simplicity's sake... Noting however that the new Business FAQ would be untranslated at that point.

    • Maybe we can get a Spanish translation of business_info in place in Spanish translation  #413 while this PR is still being worked on?
    • Redo this PR on top of develop after Spanish translation  #413 is merged for smooth sailing?
      • Alternatively, just open a new PR to translate business_info, then merge Spanish translation  #413 and hypothetical business_info translation PR, re-do this PR on top of develop
  • Paragraph text size being bumped up sounds fine once everything else is settled

    • Can help with that if desired

@mi-wood
Copy link
Member Author

mi-wood commented Jan 9, 2018

I tried to do a relative path but with the haml and locale files it was proving to be a pita so I just kinda gave up 😅

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 9, 2018

Yeah, it's more complicated than it probably should be. (That's computer tech in a nutshell for you?) :P

Here is what I came up with: DeeDeeG@6c8d7e1

@mi-wood
Copy link
Member Author

mi-wood commented Jan 9, 2018

That works :) I was trying to use the rails link helpers which made it weird

@Intimaria
Copy link
Contributor

Intimaria commented Jan 10, 2018

Hi, am having a look at DeeDeeG/refugerestrooms@6c8d7e1 and have fetched the branch locally in order to start translating it.
I noticed that there are differences between updates to about.en.yml in both your branches , one cites the app creators ( p6header: "Who all is behind this thing?") inp6headerand other links to business_info, so am not sure how to update my local file in order to update translations or maybe wait and see what happens? Both seem important.
EDIT: sorry I was looking in the wrong place.

* Add = sign to hrefs (fixes dead links)

* Change links to use relative paths (convenient for Vagrant testing)
@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 16, 2018

I think we can merge this PR if we want to clean up the PRs list a bit.

Bumping paragraph size can be posted as an issue to get to eventually, so we don't forget. Might make for a good-first-issue. OR I can try to make time to add such a paragraph size bump to about and business_info in this PR before merging this. I'd like to see this merged though, since it's so simple and rather thoroughly discussed in #272.

(note: we have 9 open PRs in the queue, all filed within the past month and a half's time! Wow, are we popular! But yeah, it's piling up!)

* Adds a class "p-text-sizebump" that scales existing font size
  of <p> elements by "1.1x". Only affects <p> elements;

  *  Style can be set directly on a <p> element, or on any ancestor
     of a <p> element, in order to have an effect on its text size.

* Applies the "p-text-sizebump" class for paragraph text within the
  "about" page, and does same within the "business_info" page.
@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 17, 2018

I added a commit that bumps paragraph size. (Computes to "15.4px" rather than "14px".) Open to comments or suggestions about how to improve the implementation.

@mi-wood mi-wood merged commit 18a0c47 into develop Jan 18, 2018
@mi-wood mi-wood deleted the update-about-and-faq branch January 18, 2018 03:06
@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 18, 2018

Glad this is merged!

By the way, business_info.en.yml ended up outside of the en sub-folder of config/locales.

Happy to move it over or let you do so, @mi-wood.

Edit: Moved it. See this commit: 75b4ba0

DeeDeeG pushed a commit to DeeDeeG/refugerestrooms that referenced this pull request Oct 15, 2018
DeeDeeG pushed a commit to DeeDeeG/refugerestrooms that referenced this pull request Nov 3, 2018
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