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

#161 Product Page: More Dates #192

Merged
merged 1 commit into from
May 16, 2019
Merged

#161 Product Page: More Dates #192

merged 1 commit into from
May 16, 2019

Conversation

ahmed-arbisoft
Copy link
Contributor

@ahmed-arbisoft ahmed-arbisoft commented Apr 29, 2019 •
edited
Loading

Pre-Flight checklist

  • Screenshots and design review for any changes that affect layout or styling
    • Desktop screenshots
    • Mobile width screenshots
    • Tag @Ferdi or @pdpinch for review

What are the relevant tickets?

Closes #161

What's this PR do?

Adds a popover on click to "More Dates" link on the Course page. This popover will show all start dates of unexpired runs.

How should this be manually tested?

Go to the page of any course with multiple unexpired runs. Click on the "More Dates" link in metadata tiles, a list should appear showing all unexpired run start dates.

Screenshots (if appropriate)

Screenshot from 2019-05-10 12-52-43
Screenshot from 2019-05-10 12-53-13

tasawernawaz
tasawernawaz requested changes Apr 29, 2019
View reviewed changes
courses/models.py Outdated Show resolved Hide resolved
courses/models.py Show resolved Hide resolved
static/scss/detail/tiles.scss Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Apr 30, 2019 •
edited
Loading

Codecov Report

Merging #192 into master will increase coverage by 0.17%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
+ Coverage   94.41%   94.58%   +0.17%     
==========================================
  Files         159      159              
  Lines        5331     5480     +149     
  Branches      312      312              
==========================================
+ Hits         5033     5183     +150     
+ Misses        242      241       -1     
  Partials       56       56
Impacted Files Coverage Δ
courses/models.py 96% <100%> (+0.06%) ⬆️
static/js/lib/ecommerce.js 100% <0%> (ø) ⬆️
static/js/lib/ecommerce_test.js 100% <0%> (ø) ⬆️
static/js/containers/pages/CheckoutPage_test.js 100% <0%> (ø) ⬆️
ecommerce/models.py 89.34% <0%> (+0.06%) ⬆️
static/js/containers/pages/CheckoutPage.js 100% <0%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fc5b3d...f25cc74. Read the comment docs.

@pdpinch
Copy link
Member

pdpinch commented Apr 30, 2019 •
edited
Loading

Here's an interim design from @abdulkdawson https://invis.io/WDRNN8N2BT8#/360581028_XPro_LandingPage_Tooltips_Dates

Note that we don't have the enroll button coding done yet. Please omit those for now.

@ahmed-arbisoft ahmed-arbisoft force-pushed the ahmedbelal/161-more-dates branch from 974ff03 to 6e1c244 Compare May 10, 2019 07:57
@ahmed-arbisoft ahmed-arbisoft changed the title More Dates Tooltip #161 Product Page: More Dates Tooltip May 10, 2019
@ahmed-arbisoft ahmed-arbisoft changed the title #161 Product Page: More Dates Tooltip #161 Product Page: More Dates May 10, 2019
asadiqbal08
asadiqbal08 suggested changes May 10, 2019
View reviewed changes
courses/models.py Outdated Show resolved Hide resolved
courses/models.py Outdated Show resolved Hide resolved
courses/templates/partials/metadata-tiles.html Outdated Show resolved Hide resolved
static/js/course_detail.js Outdated Show resolved Hide resolved
@gsidebo gsidebo mentioned this pull request May 13, 2019
Closed
7 tasks
@ahmed-arbisoft ahmed-arbisoft force-pushed the ahmedbelal/161-more-dates branch 4 times, most recently from c97b9e2 to 7ea2b96 Compare May 14, 2019 13:17
gsidebo
gsidebo suggested changes May 14, 2019
View reviewed changes
courses/models.py Show resolved Hide resolved
courses/models_test.py Show resolved Hide resolved
@gsidebo gsidebo self-assigned this May 14, 2019
@ahmed-arbisoft ahmed-arbisoft force-pushed the ahmedbelal/161-more-dates branch from 7ea2b96 to 549a8b5 Compare May 15, 2019 12:33
@ahmed-arbisoft
Copy link
Contributor Author

@gsidebo feedback addressed, please review so I can close this

@ahmed-arbisoft ahmed-arbisoft requested a review from gsidebo May 15, 2019 12:44
gsidebo
gsidebo approved these changes May 15, 2019
View reviewed changes
Copy link
Contributor

@gsidebo gsidebo left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

Feel free to merge after adding that blank line

gsidebo
gsidebo reviewed May 15, 2019
View reviewed changes
courses/models.py Outdated
@@ -301,6 +309,18 @@ def first_unexpired_run(self):
lambda course_run: course_run.is_unexpired,
)

@property
def unexpired_run_dates(self):
Copy link
Contributor

@gsidebo gsidebo May 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, one more thing. This is returning unexpired CourseRuns, not dates, so this should be called unexpired_runs not unexpired_run_dates. Feel free to merge after this change is made, tests pass, and you confirm that the UI still works

#161 Product Page: More Dates
f25cc74 
ESlint and test fixes
ESLint fixes (prettier formatting)
Popover Design
Formatting and Move to New Structure
Removed BS3 included with catalog page
Upgraded catalog page to BS4
Removed redundant CSS and JS
Modified to Include Course Run ID
Feedback from gsidebo
@ahmed-arbisoft ahmed-arbisoft force-pushed the ahmedbelal/161-more-dates branch from 549a8b5 to f25cc74 Compare May 16, 2019 09:03
@ahmed-arbisoft ahmed-arbisoft merged commit 58ef1b6 into master May 16, 2019
@ahmed-arbisoft ahmed-arbisoft deleted the ahmedbelal/161-more-dates branch May 16, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tasawernawaz tasawernawaz tasawernawaz requested changes

@gsidebo gsidebo gsidebo approved these changes

@asadiqbal08 asadiqbal08 asadiqbal08 requested changes

Assignees

@gsidebo gsidebo

Labels
Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Product Page: More Dates
7 participants
@ahmed-arbisoft @codecov-io @pdpinch @tasawernawaz @asadiqbal08 @gsidebo @odlbot