Skip to content

Implement front-end spec for Smarter Resources#563

Closed
doublevcodes wants to merge 14 commits into
python-discord:smarter-resourcesfrom
doublevcodes:doublevcodes/smarter-resources/front-end
Closed

Implement front-end spec for Smarter Resources#563
doublevcodes wants to merge 14 commits into
python-discord:smarter-resourcesfrom
doublevcodes:doublevcodes/smarter-resources/front-end

Conversation

@doublevcodes
Copy link
Copy Markdown
Contributor

@doublevcodes doublevcodes commented Aug 11, 2021

This PR will do the following things:

  • Make the following search options be ticked by default
    • General
    • Tutorial
    • Free
    • Beginner
  • Split the Topic column into two
  • Center the Search and Clear buttons
  • Link to "Edit resource on GitHub"
  • Show an error message if no options are selected

@swfarnsworth
Copy link
Copy Markdown
Contributor

Great work so far! I forgot to mention: I'd also like for there to be a specific "error message" if the user doesn't check any options for a given parameter (as this will cause an intersection with the empty set and yield no results for the whole query). If this must be handled by the backend, let me know in what way and I'll adjust it accordingly.

This is needed because the flexbox helper classes were only introduced in later versions (Bulma v0.9.1 and djano-simple-bulma v2.4.0)
`psycopg2-binary` introduced a change that cause Django to break in v2.9. Without this pin, starting up the site raises an `AssertionError: database connection isn't set to UTC`
This button should be used by maintainers to take them to the YAML file holding data about that resource. It currently has no functionality.
This newly added rule enforces exception names to end with Error or Exception, causing the code to not pass linting.

I preferred ignoring this error instead of fixing it to keep the style consistent with rest_framework.NotFound.
@doublevcodes doublevcodes marked this pull request as ready for review August 15, 2021 19:32
Comment on lines +33 to +34
"topics_1": topics[:len(topics) // 2],
"topics_2": topics[len(topics) // 2:],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to double check that this doesn't affect the back end logic in any way.

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.

It doesn't

Comment on lines +9 to +37
icons = {
"Algorithms And Data Structures": "fa-cogs",
"Data Science": "fa-flask",
"Databases": "fa-server",
"Game Development": "fa-joystick",
"General": "fa-book",
"Microcontrollers": "fa-microchip",
"Other": "fa-question-circle",
"Software Design": "fa-paint-brush",
"Testing": "fa-vial",
"Tooling": "fa-toolbox",
"User Interface": "fa-desktop",
"Web Development": "fa-wifi",
"Discord Bots": "fa-robot",
"Book": "fa-book",
"Community": "fa-users",
"Course": "fa-chalkboard-teacher",
"Interactive": "fa-mouse-pointer",
"Podcast": "fa-microphone-alt",
"Tool": "fa-tools",
"Tutorial": "fa-clipboard-list",
"Video": "fa-video",
"Free": "fa-first-aid",
"Paid": "fa-sack",
"Subscription": "fa-credit-card",
"Beginner": "fa-play-circle",
"Intermediate": "fa-align-center"
}
icon_name = icons[name]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This dictionary should be a global so that it's not being re-created each time. Try naming it _ICONS.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pretty sure Python handles this and uses LOAD CONST, that said using an all-caps variable at the top of the file is cleaner.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also I wish we had function composition so that this function could just be get_category_icon = register.filter @ 'fa {icon_name}'.format @ icons.get, but no 😞

@swfarnsworth
Copy link
Copy Markdown
Contributor

Superseded by #570, which includes all of these commits. Thanks again!

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