Comments

mherchel created an issue. See original summary.

hansa11’s picture

Assigned: Unassigned » hansa11
hansa11’s picture

StatusFileSize
new317.36 KB
new114.43 KB

@mherchel I agree, the text format block takes up too much space. If we rearrange it to something like Bartik or Classy, it would look like

Sample
The select box looks very large here, I think we should make it a bit smaller for the comments section.

We can have something like this:

Sample text format

Let me know if this would work so I'll start with the implementation or if we should get the designs for this.

Thanks!

mherchel’s picture

Agree that the select box should be smaller... but lets keep the overall style. We can also make the direction list-item font-size smaller.

hansa11’s picture

StatusFileSize
new32.4 KB

While working on this, I realized that we have attached the comments.css only for the twig comment.html.twig which is rendered only when we add a new comment and not when we Edit an existing comment.
So, the styles that are specific to comments (for instance making the select box smaller only for the comments section) will not reflect when we edit a comment.
I think we should add the comments.css in global-styling and the global classes like ".js-indented" which are in the comments.css can be wrapped in the comment specific parent class OR should we make another css file that works for global.

Please suggest!

comments

mherchel’s picture

@hansa11 - I don't want to move comments.css into global, however the styles for the "text formats" area should be global. Maybe under form.css

hansa11’s picture

Status: Active » Needs review
StatusFileSize
new16.56 KB

Patch details:
1. Smaller select box for text-area.
2. Smaller font sizes for the list items.
3. Moved the form select box variables to variables.scss.

Please review.

Thanks!

hansa11’s picture

Assigned: hansa11 » Unassigned
kostyashupenko’s picture

Status: Needs review » Needs work

Hello @hansa11 !
Thanks for your patch, but i think styles for small select shouldn't be on the level of comment-form. I will try to explain what i mean:

- Base component is form-element and it is described in form-text.css file for selector .form-element. And form-element--type-select is a just of modifier of form-element element. This modifier has only specific styles related to <select>, like arrow for example.
- that means, if you need to have smaller select, you have to provide additional modifier for form-element, like form-element--small
- then you describe new min-height property or whatever else for selector .form-element--small in form-text.css
- then if it is not enough, you have to write additional styles into form-select.css for selector .form-element--type-select.form-element--small

Now how do add form-element--small class to required select? I guess you have to alter that form and add this class from olivero.theme

kostyashupenko’s picture

Look at here:
form select

Select is a form-element. Also form-elements are input[type="text"], or textarea for example. It is all has same styles, so if you need smaller form-element, just add modifier, like form-element--small or form-element--big etc :)

kostyashupenko’s picture

Also i agree comments library shouldn't be global. If library is not loading somewhere - you can just attach it on needed preprocess hook.

hansa11’s picture

Assigned: Unassigned » hansa11

@kostyashupenko: Thanks for the review and the detailed explanation :)
I'll again take a look into this tomorrow morning.

hansa11’s picture

Assigned: hansa11 » Unassigned
Status: Needs work » Needs review
Issue tags: +Needs review
StatusFileSize
new18.49 KB

Added the modifiers & styles for text-format select box and the guidelines, please review.

Thanks!

kostyashupenko’s picture

@hansa11 thanks for your patch, gonna review it now

kostyashupenko’s picture

StatusFileSize
new45.91 KB
new61.38 KB

1. Was added OliveroPreRender class and it is implemented in /src/OliveroPreRender.php. Same as Claro theme has actually ) They also did some modifications, related to text_format.
2. About selectbox - i reworked it a little bit, i replaced form-element--type-select--small class by form-element--small. Also added Error & Disabled cases for small selectbox.
3. About filter guidelines - template of filter module was overriden, and styles for that filter applies now not globally, but from that template. Pretty similar again to how it was done in Claro.
4. Added new variable --sp0-75: calc(0.75 * var(--sp)); since it's pretty good looking with small form-elements and its paddings.

I believe we can merge it now

kostyashupenko’s picture

So now if you need somewhere small form-element, just add form-element--small class to expected textable input/select

kostyashupenko’s picture

kostyashupenko’s picture

Assigned: Unassigned » kostyashupenko
Status: Needs review » Needs work
kostyashupenko’s picture

Assigned: kostyashupenko » Unassigned
Status: Needs work » Needs review
StatusFileSize
new54.52 KB
new9.98 KB
new90.31 KB

Much improved text formats area, based on Claro.

Text format

mherchel’s picture

StatusFileSize
new54.37 KB

Re-roll

mherchel’s picture

Status: Needs review » Fixed

This looks good! I personally like when multiple background values are each on their own line (and this works with linting), but we can do this later.

Committed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.