Skip to content

Oct 2019 redesign #7

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Oct 2019 redesign #7

wants to merge 14 commits into from

Conversation

shrubin2
Copy link
Contributor

Cleaned up site, ready for desktop. Still need to make mobile version. I would double check to make sure that buttons go to the right places.

Description of Changes:

How This Was Tested:

Related Issue(s) or Specifications:

Checklist:

  • Code follows code style of this project
  • Tests added to cover changes
  • All new and existing tests passing

Questions / Additional Notes:

@vkoves vkoves self-assigned this Oct 23, 2019
@vkoves
Copy link
Contributor

vkoves commented Oct 23, 2019

@shrubin2 - can you start by resolving the conflicts? I think you're based off an older version of the site.

@vkoves
Copy link
Contributor

vkoves commented Oct 29, 2019

@shrubin2 - are their mocks for what the site should look like? It's hard for me to tell if some of the layout is intentionally the way it is.

Additionally, now that you've merged with master our CI tools have run and found a few failures. Could you take a look at the output from Travis and address those to start?

@vkoves
Copy link
Contributor

vkoves commented Oct 29, 2019

Oh, one more note: I'd suggest installing and testing stylelint locally, so you can more quickly iterate on these changes and hopefully get feedback in your editor! Otherwise it'd be pretty time consuming to fix things, push, then wait for Travis until you see if you did everything right or not.

@vkoves
Copy link
Contributor

vkoves commented Nov 1, 2019

@shrubin2 - two other notes, since I see Travis is still failing.

  1. You can run stylelint with the --fix flag to auto-fix failures. It should catch most of them! Read more here.
  2. The CI process runs a file ci.sh, so running that locally (cd into the directory than do ./ci.sh) should help you locally iterate until all the errors are fixed.

Let me know if you have any issues with those!

Copy link
Contributor

@vkoves vkoves left a comment

Choose a reason for hiding this comment

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

Just doing some manual QA, this looks good on desktop overall, but breaks on mobile. Here's a screenshot of the homepage:

Screenshot from 2019-11-14 22-48-33

Since mobile is currently working right now, I'd rather you fix the responsiveness of your changes in this PR than have this PR and then have to fix up PR #8. How does that sound?

@vkoves vkoves removed their assignment Feb 9, 2021
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.

2 participants