[PP-1 ]Image style edit page

Postponing this until the design is done.

Postponing this until the design is done.

Adding the new designs.

Ana am I missing any definition on the designs?

I think it looks good! Excited to continue working with you!

I'll be working on it later till now I'm unassigning this ticket.

Init patch

I didn't manage yarn lint:css and css-variables properly yet, so feel free to continue with it.

I'll be working on css lint and if there is something missing in design.

I've done linting of css file. Now there is no lint issue. Rest everything looks good.

Tested patch from #16 and result is following:

1. Mobile design is not provided, but i think current result is not really good - block are too small and have different size, maybe better to have these blocks each under other
2. Also, "Image style name" field has too big length and machine name should be placed under this field
3. Effect should be bold
4. Select list and Add button should be bigger with bigger margin between them

+Need design for Mobile view.

@pradeepjha as you have submitted the last patch, requesting you to work on this.

As suggested by @KondratievaS, I feel it will be good if you will put the block under the other on mobile design. I feel it will make more sense on the mobile view.

Any thoughts?

Here's the design for the mobile. Basically, one below the other as it is right now:

Also, here are the edge case on where the image is too small and how it should behave (specs here):

Tested patch from #29 and as i see for mobile resolutions content is cut - need to use horizontal scroll to see whole content
Aslo, bugs #2,3,4 from comment #18 are not fixed

Hi, I have used patch #27 and created new patch #33. Interdiff file interdiff-3072773-27-33.txt file is created as well. Fixed issue for content cut in mobile resolution (refer screenshot after-patch-mobile-screen.png).

Also tried fixing bugs #2,3,4 from comment #18 (refer screenshot after-patch-screen-resolved-comment#18.png).

Hi @shalinigaur

As I can see you have made some changes in Claro css component here:

diff --git a/core/themes/claro/css/components/button.css b/core/themes/claro/css/components/button.css
index 8594ae4390..c06718293b 100644
--- a/core/themes/claro/css/components/button.css
+++ b/core/themes/claro/css/components/button.css
@@ -253,3 +253,18 @@ a.button--danger:active {
   -moz-appearance: none;
   appearance: none;
 }
+
+
+/* specific css for image edit button */
+.image-style-new .button--extrasmall {
+  margin: 0.5rem 0.5rem 0.5rem 1rem;
+  padding: 1rem 2rem;
+  font-size: 1rem;
+}
+@media screen and (max-width: 470px) {
+  .image-style-new .button--extrasmall {
+    margin: 0.5rem 0.5rem 0.5rem 0;
+    padding: calc(0.25rem - 1px) calc(0.75rem - 1px);
+    font-size: 0.79rem;
+  }
+}

Please make below changes in your last applied patch:

1. Read my this comment https://www.drupal.org/project/drupal/issues/3098168#comment-13667256.
2. Don't override CSS .button--extrasmall better use existing some library class. We have one library class called .button--small, if you use this class then you don't have to add any css. Or add another class to write css if design is not achievable.
3. Then remove extra blank lines.
4. Please check this changes for all css changes you have done.

@pradeepjha Please see the updated patch 3072773-38.patch. Checked and made changes as mentioned. I have added a new class and removed the override css.

Working on it

Verified and tested by applying the patch #38.It looks good to me.Can be moved to RTBC.

Steps to test -
1. Go to the admin site.
2. Go to admin/appearance.
3. Select Claro theme.
4. Go to /admin/config/media/image-styles/manage/large.

After Patch Mobile -

After Patch Desktop -

Partial review:

1. +++ b/core/themes/claro/css/components/button.css
   @@ -253,3 +253,23 @@ a.button--danger:active {
   diff --git a/core/themes/claro/css/components/button.pcss.css b/core/themes/claro/css/components/button.pcss.css
   
   diff --git a/core/themes/claro/css/components/button.pcss.css b/core/themes/claro/css/components/button.pcss.css
   index f1f95222bd..0b358dcaa9 100644
   
   index f1f95222bd..0b358dcaa9 100644
   --- a/core/themes/claro/css/components/button.pcss.css
   
   --- a/core/themes/claro/css/components/button.pcss.css
   +++ b/core/themes/claro/css/components/button.pcss.css
   
   +++ b/core/themes/claro/css/components/button.pcss.css
   +++ b/core/themes/claro/css/components/button.pcss.css
   @@ -183,3 +183,21 @@ a.button--danger:active {
   

   The added styles target a specific form, and should be moved to a CSS file dedicated to that form. These styles are not comprehensive enough to be reusable elsewhere, it is largely related to positioning that is unique to the image style form.

2. +++ b/core/themes/claro/css/components/button.pcss.css
   @@ -183,3 +183,21 @@ a.button--danger:active {
   +.image-style-new .button--mediumstyle {
   +  font-size: 1rem;
   +  padding: .9rem 2rem;
   +  margin: 0.5rem 0.5rem 0.5rem 1rem;
   +}
   

   The padding and font-size styles can be removed. The font size doesn't change anything, and this padding rule provides the incorrect values. If this padding style is removed, the correct values will be inherited by other existing CSS.

   Since this means the only changes are margins, this isn't really applying a medium style anymore - the margins can be acheived using a different selector strategy.

3. +++ b/core/themes/claro/css/components/button.pcss.css
   @@ -183,3 +183,21 @@ a.button--danger:active {
   +@media screen and (max-width: 470px) {
   +  .image-style-new .button--mediumstyle {
   +    padding: calc(0.25rem - 1px) calc(0.75rem - 1px);
   +    font-size: 0.79rem;
   +    margin: 0.5rem 0.5rem 0.5rem 0rem;
   +  }
   +}
   

   The styles added here are already accomplished with .button--extrasmall. Instead of defining a new variant-like class, it's probably better to add a class like .button--add-image-style, and bring in the relvant styles for .button--extrasmall as CSS variables

4. +++ b/core/themes/claro/css/components/tabledrag.css
   @@ -288,6 +288,7 @@ body.drag {
   +  font-weight: bold;
   

   This impacts every instance of tabledrag, such as admin/structure/types/manage/article/form-display

   This should instead target the image style form, as other uses of tabledrag do not use bold here.

5. +++ b/core/themes/claro/templates/image-style-preview.html.twig
   @@ -0,0 +1,95 @@
   + * Default theme implementation to display a preview of an image style.
   

   This is a theme override, not a default implementation.

@bnjmnm Please find the updated patch, done changes as following:
Point 1. Removed the style and added it in the specific image form css.
Point 2. Font-size and padding is removed but margin is given as it was requirement mentioned to give margin between them in bugs #4 from comment #18.
Point 3. Tried doing it but the style is not working unless we make it specific for image form. Although I have renamed the class name as .button--add-image-style
Point 4. Set the css for specific image form.
Point 5. Corrected the comment

@shalinigaur,
Please try to fix the linting errors. Please have a look at the screenshot.

@komalkolekar Fixed linting errors. Please find the updated patch.

Tested patch from #45 and there are 3 issues

1. Margin is to big
2. Button and selectlist are displaced for mobile
3. For resolutions more than 470px display is not good

@KondratievaS Please find the updated patch with following changes:

1. Fixed margin (refer screenshot below)

2. Fixed Button and selectlist are displaced for mobile (refer screenshot below)

3. For resolutions more than 470px display is fixed (refer screenshot below)

If the issue still exists for any particular browser or screen-size then please let me know.

Tested patch from #52 and there are 2 bugs:

- blocks should have same size
- value in select list is cut

@KondratievaS I have re-tested on the same resolution as you tested but everything seems fine to me. I have tested on Chrome and Mozilla browsers please find the screenshots below.

Please mention the specific browser in which you are testing.

I have removed some weird code from last patch, like:

.image-style-new {
    display: ruby;
  }

or

.image-style-edit-form .js-form-type-textfield {
  display: inline-grid;
}

or

.image-style-edit-form .form-item__label {
    display: unset;
  }

and some other.

Now what was really done:
1. Added some margins / better management of cards sizes for all breakpoints.
2. It was really weird to me why we have --small modifier for buttons, but no same modifier for form-elements :) So i have added it. BUT! I'm sure we can just set selectbox + button as --extrasmall => that one will be fully enough. But for now - i have added --small modifier for form-elements. It needs discussion and careful review. Check how it looks with:
--- --small modifier

--- --small modifier

So honestly no big difference for me - we can keep extrasmall for both ?

3. I don't really think we have to manage width of that textfield, at least why? I don't see any reason (from that comment)

4. I have some doubts about mobile styling, check the following screens:
--- with hidden weights column

--- with visible weights column

Since markup is table - any solutions how we can deal with it?

It was really weird to me why we have --small modifier for buttons, but no same modifier for form-elements :) So i have added it.

There's an existing issue for that: #3083256: Create smaller variations for form elements. I'm setting this to Postponed on that issue, as some of the changes here could conflict with that. I like what I'm seeing in #58, though, so if we can get the smaller variants issue through, this could be pretty close.

The Claro theme was approved for removal in #3576460: [policy, no patch] Deprecate and remove Claro.

The deprecation work is in #3576668: [meta] Tasks to deprecate Claro and the removal work in #3584638: [meta] Tasks to remove the Claro theme.