fix: space gap between modals buttons and text color fixes #805
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @bydawen! 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. Where 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #805 +/- ##
==========================================
- Coverage 94.36% 94.36% -0.01%
==========================================
Files 167 167
Lines 3691 3690 -1
Branches 991 995 +4
==========================================
- Hits 3483 3482 -1
Misses 193 193
Partials 15 15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ihor-romaniuk
force-pushed
the
fix/discussions-sidebar-frame-buttons-gap
branch
from
817ba9e to
cfa0570
Compare
PKulkoRaccoonGang
added
the
create-sandbox
src/discussions/common/HoverCard.jsx
Outdated
| actionFunction(); | ||
| }} | ||
| className={['endorse', 'unendorse'].includes(endorseIcons.id) ? 'text-dark-500' : 'text-success-500'} | ||
| className={['endorse', 'unendorse'].includes(endorseIcons.id) ? 'text-primary' : 'text-primary'} |
There was a problem hiding this comment.
[nit]: Since the condition ['endorse', 'unendorse'].includes(endorseIcons.id) always returns true or false, and in both cases the same class text-primary is assigned, the expression can be replaced with className="text-primary". What do you think about this?
There was a problem hiding this comment.
[nit]: Since the condition
['endorse', 'unendorse'].includes(endorseIcons.id)always returnstrueorfalse, and in both cases the same classtext-primaryis assigned, the expression can be replaced withclassName="text-primary". What do you think about this?
✅ Good catch, thanks! I removed the redundant condition and replaced it with className="text-primary".
src/index.scss
Outdated
| z-index: 5000 !important; | ||
|
|
||
| .pgn__modal-footer .pgn__action-row { | ||
| gap: 8px; |
There was a problem hiding this comment.
Can we use the variable --pgn-spacing-spacer-2 instead of 8px?
Another question is more global, it seems to me that we should fix this problem not in this MFE, but in Paragon (since it is global and refers to the ActionRow component of Paragon). I tried to see how ActionRow looks on mobile screens and noticed the following
Can you create a corresponding issue for this problem in Paragon? If you have other ideas, feel free to share them
There was a problem hiding this comment.
I think it must be fixed on the paragon side, because this problem present in that component, and will be everywhere where it will be used.
Created issue in the paragon repo:
openedx/paragon#3775
|
Sandbox deployment failed 💥 |
|
Sandbox deployment failed 💥 |
|
Sandbox deployment successful 🚀 |
bydawen
force-pushed
the
fix/discussions-sidebar-frame-buttons-gap
branch
from
cfa0570 to
54806e2
Compare
|
Sandbox deployment failed 💥 |
|
Sandbox deployment successful 🚀 |
bydawen
force-pushed
the
fix/discussions-sidebar-frame-buttons-gap
branch
from
54806e2 to
6cde837
Compare
|
Sandbox deployment successful 🚀 |
bydawen
force-pushed
the
fix/discussions-sidebar-frame-buttons-gap
branch
from
6cde837 to
9db46d6
Compare
…sections text colors
bydawen
force-pushed
the
fix/discussions-sidebar-frame-buttons-gap
branch
from
9db46d6 to
2ec8d8b
Compare
|
Sandbox deployment successful 🚀 |
5 participants
Description
There is an issue, if open discussions in the sidebar when in course. And then choose dropdown with the actions, then report or delete, in the appeared modal focus on buttons will be overlaped on each other:
This fix add some gap, to prevent this style issue.
Additional information
Additionally fixed styles for the comment text in the reply/post sections, currently it has wrong classes for the text-color:
For now text in there has
text-primary-500which may have incorrect perception as it will look like the main color of the platform instead of regular text color and not being an active element.Testing Instructions
Changelog
text-primary-*color clases to regulartext-gray-*on text elements in the post sections.