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.

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

Added waffle flag state for Courseware Search #1199

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

rijuma
Copy link
Member

@rijuma rijuma commented Oct 4, 2023

Description

Ticket: KBK-36 (internal)
This relates to an edx-platform PR which adds the endpoint to the LMS (openedx/edx-platform#33372)

There's a project to add edx-search into the course home. Right now I'm creating a placeholder button which will respond to the Course Waffle Flag toggle of courseware.mfe_courseware_search.

UI changes

search-placeholder

If the flag is enabled, a search button will be shown on the course's home navigation. I'm using the float property to position itself, which might sound controversial. This was the easiest way to achieve it since the CourseTabsNavigation component has a wrapping logic whenever the navigation items don't fit. I've tried to move the navigation to a flex container but it just messes up with this logic. Luckily, the button is small enough so it does not interfere with the remaining space after folding.

search-plaheholder-2

Testing Instructions:

Navigate to the LMS Admin and toggle courseware.mfe_courseware_search flag via:

  • Waffle Flags: http://(...)/admin/waffle/flag/
  • Course Waffle Flags: http://(...)/admin/waffle_utils/waffleflagorgoverridemodel/

Results:

You should be able to see the search placeholder or not (see screenshot above) depending on the enabled state for the Waffle Flag.

@rijuma rijuma changed the title Added waffle flag state for Courseware Search Added waffle flag state for Courseware Search (wip) Oct 4, 2023
@rijuma rijuma force-pushed the rijuma/courseware-search-waffle-flags branch 3 times, most recently from d2a5202 to c3eaded Compare October 10, 2023 14:05
@rijuma rijuma self-assigned this Oct 10, 2023
@rijuma rijuma force-pushed the rijuma/courseware-search-waffle-flags branch 8 times, most recently from a65fb4b to e99fc14 Compare October 11, 2023 21:05
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (570cdb4) 88.05% compared to head (2d243a5) 88.09%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1199      +/-   ##
==========================================
+ Coverage   88.05%   88.09%   +0.04%     
==========================================
  Files         266      268       +2     
  Lines        4519     4537      +18     
  Branches     1143     1145       +2     
==========================================
+ Hits         3979     3997      +18     
  Misses        526      526              
  Partials       14       14              
Files Coverage Δ
...course-home/courseware-search/CoursewareSearch.jsx 100.00% <100.00%> (ø)
src/course-home/courseware-search/messages.js 100.00% <100.00%> (ø)
src/course-home/data/api.js 90.00% <100.00%> (+0.20%) ⬆️
src/course-home/data/thunks.js 78.57% <100.00%> (+2.10%) ⬆️
src/course-tabs/CourseTabsNavigation.jsx 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rijuma rijuma marked this pull request as ready for review October 11, 2023 21:20
@rijuma rijuma changed the title Added waffle flag state for Courseware Search (wip) Added waffle flag state for Courseware Search Oct 11, 2023
@rijuma rijuma force-pushed the rijuma/courseware-search-waffle-flags branch from e99fc14 to 30a0f3f Compare October 11, 2023 21:29
Copy link
Contributor

@schenedx schenedx left a comment

Choose a reason for hiding this comment

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

👍

@rijuma rijuma force-pushed the rijuma/courseware-search-waffle-flags branch from 30a0f3f to 2d243a5 Compare October 12, 2023 15:10
@rijuma rijuma merged commit 62465ec into master Oct 12, 2023
6 checks passed
@rijuma rijuma deleted the rijuma/courseware-search-waffle-flags branch October 12, 2023 19:06
CefBoud pushed a commit to open-craft/frontend-app-learning that referenced this pull request Nov 5, 2023
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