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

SHS-5582: Bug: Issue with Add/Select Media form field #1511

Merged
merged 5 commits into from
May 2, 2024

Conversation

mariannuar
Copy link
Collaborator

@mariannuar mariannuar commented Apr 23, 2024

READY FOR REVIEW

Summary

Fix the "Sort by" label and form field that has incorrect styling when site editor uses the Media Library in CKEditor

Need Review By (Date)

04/03

Urgency

low

Steps to Test

  1. Create a Flexible Page on Colorful and Traditional
  2. Select the Media Library in CKEditor and confirm the "Sort by" label and form field looks like this
  3. Confirm any select/combox still looks as expected in other forms (Views for example)

PR Checklist

@mariannuar mariannuar self-assigned this Apr 23, 2024
@mariannuar mariannuar requested a review from cienvaras April 23, 2024 23:56
@cienvaras cienvaras changed the base branch from develop to fk-stnfd-sprint-49 April 26, 2024 21:00
Copy link
Collaborator

@cienvaras cienvaras left a comment

Choose a reason for hiding this comment

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

@mariannuar Thanks! Some fixes needed though, please check the inline comments.

@mariannuar
Copy link
Collaborator Author

@cienvaras Thank you for your feedback! Now the preact plugin is looking similar to the regular select.

@mariannuar mariannuar requested a review from cienvaras April 26, 2024 23:22
Copy link
Collaborator

@cienvaras cienvaras left a comment

Choose a reason for hiding this comment

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

@mariannuar We're almost there, but there are still some inconsistencies that can be easily fixed:

  • Label's line height should be 18px
  • Button's vertical padding should be 7px, instead of 8px
  • Border radius is smaller than other elements

Please use Gin CSS variables when available.

@@ -97,4 +97,4 @@ drupal-media {
// it causes overflow into the next column
.ck-editor .ck.ck-toolbar.ck-toolbar_grouping > .ck-toolbar__items {
flex-wrap: wrap;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this change.

body.gin--edit-form .views-exposed-form .select-preact div > div > div:first-of-type {
font-size: 0.875rem !important;
font-weight: 525 !important;
color: #222330;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Gin has variables for most if its properties, please use them when available. In this case, var(--gin-color-title).

@mariannuar
Copy link
Collaborator Author

@cienvaras Thanks! Using gin variables now.

@mariannuar mariannuar requested a review from cienvaras April 30, 2024 16:53
Copy link
Collaborator

@cienvaras cienvaras 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, thanks! 🚀

@cienvaras cienvaras merged commit 85b316b into fk-stnfd-sprint-49 May 2, 2024
13 of 15 checks passed
@cienvaras cienvaras deleted the SHS-5582 branch May 2, 2024 13:10
joegl pushed a commit that referenced this pull request May 6, 2024
* SHS-5608: Collection: Title Settings field updates (#1512)
* SHS-5582: Bug: Issue with Add/Select Media form field (#1511)
* SHS-5613: Bug: Vertical Link Cards with /media/{id} URLs not showing URL to file (#1518)
* SHS-5574: Implementation: Refactor some grid CSS/mixins (#1508)
* SHS-5571: A11y Level A: Empty headings on Vertical Timeline components (#1502)
* SHS-5568: Allow editors to select style on Accordion component (#1501)
* SHS-5583: empty headings in banners and spotlights (#1510)
* SHS-5612: Implementation: Vertical Linked Card Title/Arrow Updates (#1513)
cienvaras added a commit that referenced this pull request May 16, 2024
* SHS-5608: Collection: Title Settings field updates (#1512)

* feat(shs-5608): update hs_collection title settings field

* feat(shs-5608): open hs collections title settings link in new tab

---------

Co-authored-by: Andrés Díaz Soto <[email protected]>

* SHS-5582: Bug: Issue with Add/Select Media form field (#1511)

* fix(shs-5582): issue with Add/Select Media form field

* fix(shs-5582): make the preact-select look like the default one

* fix(shs-5582): fix more styles

* feat(shs-5582): use gin variables

* feat(shs-5582): use gin variables

---------

Co-authored-by: Mari Núñez <[email protected]>

* SHS-5613: Bug: Vertical Link Cards with /media/{id} URLs not showing URL to file (#1518)

* feat: fix handling of Drupal Routes in basic theme

* feat: remove unused use statements

* feat(shs-5623): updates on date stacked vertical card

* feat(shs-5623): reuse same code

* fix(shs-5623): more fixes

---------

Co-authored-by: Hector Lopez <[email protected]>
Co-authored-by: Andrés Díaz Soto <[email protected]>
Co-authored-by: Mari Núñez <[email protected]>
joegl pushed a commit that referenced this pull request May 20, 2024
* SHS-5623: Update styles for Date Stacked Vertical Card pattern (#1519)
* SHS-5608: Collection: Title Settings field updates (#1512)
* SHS-5582: Bug: Issue with Add/Select Media form field (#1511)
* SHS-5613: Bug: Vertical Link Cards with /media/{id} URLs not showing URL to file (#1518)
* SHS-5626: Implementation: New Accordion Style (FED) (#1525)
* SHS-5581: Remove Chosen plugin dependency (#1530)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants