Skip to content
This repository was archived by the owner on Aug 8, 2019. It is now read-only.

Feature Owner Home UI#11

Merged
cristianbgp merged 3 commits intodevelopfrom
feature-ownerhomeui
Jul 12, 2019
Merged

Feature Owner Home UI#11
cristianbgp merged 3 commits intodevelopfrom
feature-ownerhomeui

Conversation

@cristianbgp
Copy link
Copy Markdown
Contributor

@cristianbgp cristianbgp commented Jul 9, 2019

This PR adds

  • UI for the Owner Home view.
  • List of clubs from the API
  • List of sport fields from the API
  • UI for the button to create a new club or sport field
  • Also there is a ui.js file where anyone can create generic styled components

Close #5
Close #6
Close #7

@cristianbgp cristianbgp force-pushed the feature-ownerhomeui branch from bab1372 to d609f4d Compare July 10, 2019 14:15
@cristianbgp cristianbgp force-pushed the feature-ownerhomeui branch from 376d67a to 82c1195 Compare July 11, 2019 23:21
Comment thread api/app/controllers/clubs_controller.rb Outdated
end

def club_params
params.permit(:name, :email, :city, :country, :address, :image, gallery: [])
Copy link
Copy Markdown

@condef5 condef5 Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Could you remove this line please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I'll do it

Comment thread client/src/views/owner-home.js Outdated
}, [setClubs]);

React.useEffect(() => {
getSportFields().then(sportFields => {
Copy link
Copy Markdown

@condef5 condef5 Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you should validate that the active club is different from null for
bring the sports fields for each club and avoid making unnecessary calls to the server:

 React.useEffect(() => {
    if (activeClub) {
      getSportFields(activeClub).then(sportFields => {
        setSportFields(sportFields);
      });
    }
  }, [setSportFields]);

Comment thread client/src/views/owner-home.js Outdated
const sportFields = useSportFields();
const setClubs = useSetClubs();
const setSportFields = useSetSportFields();
const [activeClub, setActiveClub] = React.useState(1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the default value should be null, because if there are no clubs, it means that there are no sport fields either and when you make the request to get the sports fields it will give error:

const [activeClub, setActiveClub] = React.useState(null);


React.useEffect(() => {
getClubs().then(clubs => {
setClubs(clubs);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could pass the default value of the club(activeClub) here:

React.useEffect(() => {
    getClubs().then(clubs => {
      setClubs(clubs);
      setActiveClub(clubs.length ? clubs[0].id : null)
   });
 }, [setClubs]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing all of that, I'll change it right now.

@cristianbgp cristianbgp dismissed a stale review via 50d3542 July 12, 2019 00:58
@cristianbgp cristianbgp merged commit d36a1a1 into develop Jul 12, 2019
@KattyaCuevas KattyaCuevas deleted the feature-ownerhomeui branch July 16, 2019 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants