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.

Sign up for GitHub

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

Jump to bottom

Web39 - update based on student feedback #101

Merged
merged 3 commits into from
Mar 26, 2021
Merged

Web39 - update based on student feedback #101

merged 3 commits into from
Mar 26, 2021

Conversation

wlongmireBloomTech
Copy link
Contributor

After surveying students for false negatives on codegrade tests (26 self reporting out of 68 students), the following adjustments have been:

  1. Made sure color component is explicitly use to avoid problems with counting colors in initial tests.
  2. Added a preset error p tag tagged with a testid for more explicit testing.
  3. Added username and password for the server auth into the project instructions.
  4. Mentions all calls that require auth in the project instructions
  5. Added a logout button and permeant header to facilitate testing.
  6. Made delete button always visible regardless of edit mode.
  7. Remove unneeded useEffect in App.js/
  8. Added instructions to make sure students explicitly add data-testid="username" and data-testid="password" to their login forms.

dustinmyers
dustinmyers approved these changes Mar 3, 2021
View reviewed changes
Copy link

@dustinmyers dustinmyers left a comment

Choose a reason for hiding this comment

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

Looks good from my end. Just take a look at that one line in the README that seems like it should be removed.

dustinmyers
dustinmyers reviewed Mar 3, 2021
View reviewed changes
README.md
* [ ] Create a new branch: git checkout -b `<firstName-lastName>`.
* [ ] Implement the project on your newly created `<firstName-lastName>` branch, committing changes regularly.
* [ ] Push commits: git push origin `<firstName-lastName>`.
* [ ] Create a new branch with the
Copy link

Choose a reason for hiding this comment

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

This line looks like it could be removed 😁

@wlongmireBloomTech wlongmireBloomTech merged commit c47e18e into main Mar 26, 2021
@wlongmireBloomTech wlongmireBloomTech deleted the update branch March 26, 2021 15:43
Ladrillo pushed a commit to Ladrillo/web-sprint-challenge-advanced-web-applications that referenced this pull request Dec 17, 2021
@wlongmireBloomTech
Merge pull request bloominstituteoftechnology#101 from LambdaSchool/u…
98c11e0 
…pdate

Web39 - update based on student feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dustinmyers dustinmyers dustinmyers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wlongmireBloomTech @dustinmyers