Repository navigation
feat: small changes to improve UI - #636
Conversation
|
Thanks for the pull request, @jesusbalderramawgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
I'm not rendering the CCX ID anymore because it is too long and I didn't consider it was necessary but let me know your thoughts on this, maybe it was there for a reasion. |
e18f439 to
3afd942
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #636 +/- ##
==========================================
- Coverage 98.60% 98.60% -0.01%
==========================================
Files 112 112
Lines 1220 1219 -1
Branches 209 207 -2
==========================================
- Hits 1203 1202 -1
Misses 16 16
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@arbrandes, thank you and sorry about the imports I made a rebase and I messed up the imports, sorry for the inconveniences. I have addressed your comments, thank you@ |
arbrandes
left a comment
There was a problem hiding this comment.
It's fine not to have the course ID in CCX, but we need to show something in the standalone gradebook to say what course the user is in. Ideas?
Also, a few further findings inline.
| const deriveCourseName = (courseId) => ( | ||
| (courseId?.split(':')[1] ?? '').split('+').slice(0, 2).join(' ') | ||
| ); |
There was a problem hiding this comment.
This isn't the course name, it's org + course number: course-v1:TestU+CS101+2024 becomes "TestU CS101". Dropping the run means every run of a course shows the same label, which is the one thing a course identifier in the header should disambiguate.
Either show the real display name (the instructor dashboard gets it from its course info API, see CourseInfoSlot), or keep it simple and render the full courseId in smaller text below the heading. Long CCX ids can wrap with text-break, as before.
| <Bubble className="mr-2">1</Bubble> | ||
| <h3 className="h4 text-primary-700 my-0"> | ||
| <span>{formatMessage(messages.filterStepHeading)}</span> |
There was a problem hiding this comment.
Nit: now that the Bubble is outside the h3, the inner <span> does nothing and can go. Same at 58-60. The bare <div> wrapper at 47 is also left over from the ml-3 and can be dropped.
| import PropTypes from 'prop-types'; | ||
|
|
||
| import { useIntl } from '@openedx/frontend-base'; | ||
|
|
||
| import { Bubble } from '@openedx/paragon'; | ||
| import { useFilters } from '@src/data/filtersContext'; |
There was a problem hiding this comment.
Nit: restore the blank lines between import groups (third-party, @src, local). The other files in this PR, including GradebookHeader/index.jsx, keep them.
|
thank you @arbrandes, I brought back the courseId and followed your suggestion. |
arbrandes
left a comment
There was a problem hiding this comment.
Ok, let's get this in! Thanks!
|
🎉 This PR is included in version 2.0.0-alpha.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |


Description
This project was recently upgraded to frontend-base, and we found out that we need to make some refactors and improve the UI, Currently is not in the best shape.
in the future this repo will need a refactor to improve the code and also to have a better UI and make it match with all the new styles.
What does this PR do?
I made a small change while we work on the refactor to make the UI a little bit better, we are using this frontend application inside the instructor dashboard so it is worth it to make this small changes.
Screnshots
Before
After