These are chat archives for CodeHubOrg/discussions

23rd
Mar 2017
Tim Handy
@TimHandy
Mar 23 2017 07:33
@rinse0ut Roger.
Tim Handy
@TimHandy
Mar 23 2017 07:48
@rinse0ut I pulled from master git pull upstream master for javascript101. That's correct? we're not using a branch or git-flow stuff right? There are two other remote branches... from last js101 meetup. Shall I remove them? I think they might be confusing.
Dave Thomson
@rinse0ut
Mar 23 2017 07:49
You can delete dev but no the holding page as that is whats on javascript101.co.uk at the moment
@TimHandy
Yes pull from master
Dave Thomson
@rinse0ut
Mar 23 2017 07:55
You should follow the make a pull request guide and make a feature branch locally
Tim Handy
@TimHandy
Mar 23 2017 07:57
ahh, yes. thanks.
Katja Durrani
@katjad
Mar 23 2017 10:15
Wow so much on the home page now, really good. thanks for doing all that @rinse0ut. I am not sure when I'll get round to our bit, hopefully at hack night latest
Tim Handy
@TimHandy
Mar 23 2017 10:16
@rinse0ut right... got it pulling data from the github api and creating cards (temporarily using test data as hitting api limit). I'm finding that my cards are not wrapping correctly and not sure of the best way to get them to do this... set an exact height on the cards, or a clearfix somehow?
Tim Handy
@TimHandy
Mar 23 2017 10:21
should I be messing with overriding bootstrap's css media queries to add a clearfix on nth-of-type(4) to make the 4th item clear at that media query width? That's the only thing I've seen before.
Dave Thomson
@rinse0ut
Mar 23 2017 10:32
@TimHandy Nice one. You could try using the masonry plugin. I’ve used it before and it animates the tiles on window resize. http://stackoverflow.com/questions/32135519/how-can-i-create-tile-layout-in-bootstrap
Tim Handy
@TimHandy
Mar 23 2017 11:16
@rinse0ut ok. will check it out.
cheers
Tim Handy
@TimHandy
Mar 23 2017 11:56
@rinse0ut right... we're in business! I don't think you're gonna like that my projects.js file is currently sat in the public dir, but maybe that's something we can figure out later as a reorg/refactor exercise? I couldn't get it to work without being there... something to do with the Express static share only sharing 'public' maybe? I'm so used to webpack sorting this out now I haven't a clue how to do it this way.
Dave Thomson
@rinse0ut
Mar 23 2017 11:57
Sweet! No worries we can do the house keeping later.
Tim Handy
@TimHandy
Mar 23 2017 12:00
PR made
Dave Thomson
@rinse0ut
Mar 23 2017 12:00
:+1:
Dave Thomson
@rinse0ut
Mar 23 2017 12:22
@TimHandy Nice work! Just a bit of house keeping to do please:
  • Add a h1 project title
  • Move projects.js to a new js folder in public
  • Include projects.js with <script src="js/projects.js"></script>
  • Save and include jquery from js folder
  • Move testApiResponse data to bottom of file and comment out with the other test code
  • Use one line of white space between functions in project.js
  • Change makeCard to take a a single ‘project’ param and use es6 destructoring in the function to get define projName etc
  • Change projectsUrl and githubUrl to capitals with underscores eg GITHUB_URL to show they are constants
  • Add css to styles.css to remove top margin on h3
Tim Handy
@TimHandy
Mar 23 2017 12:23
cheers... ok... roger... I'm on it...
Dave Thomson
@rinse0ut
Mar 23 2017 12:24
:+1:
Dave Thomson
@rinse0ut
Mar 23 2017 12:29
@TimHandy Added a couple more todos
Tim Handy
@TimHandy
Mar 23 2017 12:30
i saw :-) I'm working through them... I like this way of working...
Dave Thomson
@rinse0ut
Mar 23 2017 12:30
Right on Jon!
Dave Thomson
@rinse0ut
Mar 23 2017 12:36
@TimHandy Also add masonry as a static asset and include at bottom of html file
Tim Handy
@TimHandy
Mar 23 2017 12:38
@rinse0ut Ahh, that makes sense... done that one.
I commented out the 60px body top margin in styles.css as it added whitespace above the nav. It hasn't seemed to have affected the other pages.
ignore my last... resolved and put back the css to orginal.
Tim Handy
@TimHandy
Mar 23 2017 13:33
@rinse0ut Requested changes made, bar the destructuring one... though I did refactor to take the projects obj which was much neater. Thanks for that. I think the destructuring in this case makes it less readable.
it looks like it already knows to add my recent changes to the original PR.
Dave Thomson
@rinse0ut
Mar 23 2017 14:18
@TimHandy re: destructoring
How were you going to do it?

Instead of this

const card = makeCard(proj.name, proj.description, proj.html_url, proj.homepage

I was going to suggest this:

const card = makeCard(project)
and this:
const makeCard = (projName, description, appUrl, homepage) => {

  const desc = description || 'No Description found'
  const home = homepage || github

  const card = `<div class="col-sm-6 col-md-4 grid-item">
        <div class="thumbnail">
          <div class="caption">
            <h3>${projName}</h3>
            <p>${desc}</p>
            <p><a href=${appUrl} class="btn btn-default" role="button">App</a> <a href=${home} class="btn btn-default" role="button">Repo</a></p>
          </div>
        </div>
      </div>`

  return card
}
to this:
const makeCard = (project) => {

  const { projName, description, appUrl,  homepage } = project
  const desc = description || 'No Description found'
  const home = homepage || github
…
Dave Thomson
@rinse0ut
Mar 23 2017 14:23
I like the project show case :sunglasses:
Nice work Tim. Merged! :shipit: :smile: :+1:
Dave Thomson
@rinse0ut
Mar 23 2017 14:48
@TimHandy Just commented on improving the formatting on the home page thanks.
Tim Handy
@TimHandy
Mar 23 2017 15:02
ahh, i see... i thought i would have had to do all of that destructuring in a single line and not three. Yours makes sense, saves a line. I'll change it.
will look at the comment also. I've noticed the Issues you've assigned to me... I like that, didn't know that was a thing in github.
Dave Thomson
@rinse0ut
Mar 23 2017 15:04
Nice one. You can also do it with arrays like this:
const { foo, bar, baz } = [ ‘first’, ‘second’, ‘third’ ]
Tim Handy
@TimHandy
Mar 23 2017 15:05
thats assigning the value 'first' to the key 'foo' ?
Dave Thomson
@rinse0ut
Mar 23 2017 15:05

Yes assigning it the the variable foo

foo  ===  ‘first’
…

We’ll have to set up some custom tags for priorites and difficultly level on the issues

Tim Handy
@TimHandy
Mar 23 2017 15:06
ok, that's confusing. :-/
for my lickle brain.
Dave Thomson
@rinse0ut
Mar 23 2017 15:06
Basically it works in the same way, perhaps first second etc makes it confusing. Will have to make a better example later
Tim Handy
@TimHandy
Mar 23 2017 15:08
ok, no worries.

on the last destructuring thing... i think i must have refactored....

const makeCard = (project) => { 

  const projName = project.name
  const description = project.description || 'No Description found'
  const app = project.homepage || project.html_url
  const repo = project.html_url

I don't see how a destructuring line there would be of benefit, as each one would need to be assigned again anyway as I gave them either a more relevant name, or included a default fallback. Or maybe I'm missing the point?

Dave Thomson
@rinse0ut
Mar 23 2017 15:10
  • I edited the comment above it assigns it the a var foo not an array key
Personally I think it makes it nicer to read as it removes the dot notation.
Tim Handy
@TimHandy
Mar 23 2017 15:12
ok, I'll try it.
I'll try your foo bar baz in devtools and see what it does.
Dave Thomson
@rinse0ut
Mar 23 2017 15:13
Say for example you used promise.all which returns an array of promises, you could use it to assign vars to each array value.
Tim Handy
@TimHandy
Mar 23 2017 15:42
@rinse0ut I can't get your foo bar baz example to work in chrome, even if I change the quotes to valid chars. Are you sure that's right? I can't find examples with an object on one side and an array on the other.
I don't know how to keep these changes to one per PR... it automatically groups any additional changes I make into the same PR without me doing anything.
Dave Thomson
@rinse0ut
Mar 23 2017 15:45
Oops no curls:
const [ foo, bar, baz ] = [ 'first', 'second', 'third' ];
Jack Leigh
@leighman
Mar 23 2017 15:46
@TimHandy You can also give defaults when destructuring - http://es6-features.org/#FailSoftDestructuring
Tho re. the above the names have to be the same to do the shorthand destructuring
Tim Handy
@TimHandy
Mar 23 2017 15:48
@rinse0ut Ahh yes, now it works as I initially thought it should. cheers.
Jack Leigh
@leighman
Mar 23 2017 15:50
You'd actually have to have:
const makeCard = ({name: projName, description, html_url: appUrl, homepage}) => {
...
Dave Thomson
@rinse0ut
Mar 23 2017 15:50
:+1:
Jack Leigh
@leighman
Mar 23 2017 15:50
(You can destructure in the argument list too)
Tim Handy
@TimHandy
Mar 23 2017 15:51
@leighman I tried the default for const desc = description || 'No description found!' to adding description = 'No description foundto the destructured version, but it didn't work, 'null' was assigned instead.
oh... colon?
can you use = and : interchangably? the example shows it's an equal symbol.
Jack Leigh
@leighman
Mar 23 2017 15:52
const {description = "No description found"} = project will work
Tim Handy
@TimHandy
Mar 23 2017 15:53
@leighman that's exactly what I tried... I'll try it again though.
Jack Leigh
@leighman
Mar 23 2017 15:53
or (I assume) const {description: desc = "No description found"} = project where you want the variable to be called desc
: is rename and = is default
Tim Handy
@TimHandy
Mar 23 2017 15:56
Nope, const {description = "No description found"} = project just resulted in returning 'null'. I changed the var in the template to use 'description'. const {name, description = "No description found!", html_url, homepage} = project
Jack Leigh
@leighman
Mar 23 2017 15:56
So the above is read Take description from project calling it desc and using the string "No description found" if it's null
Tim Handy
@TimHandy
Mar 23 2017 15:56
ahh, rename and default... clever. thanks, didn't know that.
Jack Leigh
@leighman
Mar 23 2017 15:56
Using babel?
Tim Handy
@TimHandy
Mar 23 2017 15:57
could be the problem i suppose... let me check caniuse
Default is only if key is undefined too
That could be your problem
Tim Handy
@TimHandy
Mar 23 2017 16:00
ahh, it's not undefined, it's null
destructuring is all good in chrome. must be the undefined thing. thanks Jack.
Tim Handy
@TimHandy
Mar 23 2017 16:11
 const {description, homepage, name: projName, html_url: repo} = project
  const desc = description === null ? 'No description found!' : description
  const app = homepage || project.html_url
that works!
could have shortened line two to const desc = description || 'none found' but I think this is clearer.
Jack Leigh
@leighman
Mar 23 2017 16:17
Line 2 won't catch undefined anymore where || will
(dunno whether you care)
and line 3 project.html_url is now repo
Tim Handy
@TimHandy
Mar 23 2017 16:24
ahh, good eyes.. thanks. The undefined doesn't matter as the github api returns null. the other one does matter!
@rinse0ut before you tell me off, I saw your message about separate branches... but i'd already started ... hence it still looks like i'm doing it! sorry.
Tim Handy
@TimHandy
Mar 23 2017 16:36
So, let's say I've just done a PR for a feature, and it's not yet been merged (or I don't know whether it will be approved or not), I shouldn't merge my local feature-branch in my master and delete my local feature-branch? and there's no point in re-pulling from upstream yet. Do I just branch off my feature-branch if i want to work on another feature? Git gets hella complex when more people are involved.
Jack Leigh
@leighman
Mar 23 2017 16:40
If your new feature depends on something in the branch that is waiting to merge then branch off that, if not then pull master again and branch off that
There will always be some tidying up to do (usually before you push) - get familiar with rebase ;)
Dave Thomson
@rinse0ut
Mar 23 2017 17:00
@TimHandy No worries. Merged. Go jumbotron! :ship:
Dave Thomson
@rinse0ut
Mar 23 2017 17:09
Well done Tim for completing the first two issues today! :tada: Awesome work! :sunglasses:
@leighman Thanks for the ES6 advice!
Tim Handy
@TimHandy
Mar 23 2017 17:47
@rinse0ut cheers. it's been good, i've learnt some more git, implemented a few things i've not done before, got some great advice from you and jack
Jack Leigh
@leighman
Mar 23 2017 17:59
@TimHandy and I did a Codewars Kata in Glitch with Hangouts. Fun! :thumbsup: