🇮🇳India @sandip

Account created on 20 September 2024, 7 months ago
#

Recent comments

🇮🇳India sandip

Hi @b.khouy, but i am not able to reproduce this issue in Gin also. Can you verify if i am missing something.

🇮🇳India sandip

Yes @jphelan, this issue is reproducable and the css is coming from claro which is overriding gin styles. i am attaching SS for reference.
I am working on it.

🇮🇳India sandip

The MR looks good to me input tags are now inside the container not overflow from the container. So moving this issue to RTBC.

🇮🇳India sandip

Hi @snehal-chibde, your question sounds good to me.

@catch, @nicxvan can you please guide here should i create tablesort.css inside claro/css/components folder or the above MR using claro/css/classy/components/tablesort.css is good to go ?

🇮🇳India sandip

Please review the changes.

I removed this piece of code as i think we dont need this part and also tested in local.

.sortable-heading > a:focus,
.sortable-heading > a:hover {
  -webkit-text-decoration: none;
  text-decoration: none;
}
🇮🇳India sandip

Changes looks good to me attaching a Video for better understanding. Also corrected the branch versions in CR. Moving this issue to RTBC.

🇮🇳India sandip

Hi @julio_retkwa, I’ve reviewed your MR — the changes are looking good overall. One suggestion I’d like to propose is to enhance accessibility by adding the aria-hidden attribute to the <span> elements wrapping the hamburger and close icons.

In addition, we should update the JavaScript to toggle the aria-hidden values based on the menu state. This ensures that only the currently visible icon is announced by screen readers, aligning with accessibility best practices.

Let me know what you think!

🇮🇳India sandip

https://www.drupal.org/project/drupal/issues/3068696 I found this issue which is already working for fixing the table responsive issue for claro theme. I think we can close or Duplicate this issue. @smustgrave, can you please look into it.

🇮🇳India sandip

Fixed the pipeline now the MR is good to go.

🇮🇳India sandip

Yes @piridium, it is RTBC but we need to compile the SCSS properly.
Use npm install and then npm run build. If someone want fix it they can follow this step. Otherwise I am not available right now but will do it tomorrow.

🇮🇳India sandip

This is the issue that i was talking about see the attached image.

🇮🇳India sandip

Hi @mudasirweb, the overlapping issue is resolved by the MR 608 and align-self: center is used here to center the drag button that is totally correct. Now you are coming with the issue of responsive that is okay but the previous MR 548 fixes the responsive issue but it brings more issues that @piridium mentions in the MR with a image. I think we can go with the MR 608 it is fine. If we want to fix the responsive issue i observed width: 1px is applied to the td.file-operations-cell. This causes the input tag inside it overflow. Now this css is coming from claro. If somehow we remove this width: 1px then our job is done. See the image i have attached then it will be clear.

But in my point of view the main motive of this issue was to resolve the overlapping issue See this image and that gets fixed by the MR 608.

cc: @piridium

🇮🇳India sandip

I attempted to reproduce the issue but was unable to do so from my end. I believe I need to explore the theme and its workflow further to better understand the context. In the meantime, I’m unassigning myself so that others can contribute if they’re interested.

🇮🇳India sandip

Please review the code changes. I have added a check to see if both the sticky header and normal table header are rendered. If so, I’m setting tabindex="-1" on the normal header elements to avoid duplicate tab stops. Let me know if it looks good or needs changes.

🇮🇳India sandip

Changes in the MR looks good. Attaching before and after Screenshot and moving to RTBC.
Before:

After:

🇮🇳India sandip

I am reviewing this one as there is no response from @vasantha for more than 1 week.

🇮🇳India sandip

Sorry my bad! that thing is only works for table.html.twig not working for view-view-table.html.twig. Sorry for the above noice i did not notice it before. I am fixing it.

🇮🇳India sandip

Hi @nod_ thanks for the review. I wanted to clarify that I can see the table.css file being added correctly. For testing purpose, I intentionally added background-color: blue to the table tag, and I verified in the Network tab also that the stylesheet is loading as expected. Can you again have a look at it please.

For reference, I’ve attached a screenshot:

Thanks again for your guidance and for providing these valuable resources. I haven’t try any recordable changes yet, but I’m going through the documentation you shared and trying to implement the same here.

🇮🇳India sandip

Thanks @wim leers, for the correction. Thanks @meghasharma, for the update. Now it needs just little change in
jsonSchema: $client_side_info['field_data'][$prop]['jsonSchema'], and \assert(isset($client_side_info['field_data'][$prop]['jsonSchema']));. I am working on it.

🇮🇳India sandip

I've noticed that the extra tab through the table header occurs because two table headers are being rendered—one for the sticky header and another for the regular table header. To resolve this, I am considering keeping the tab focus on the sticky header while removing it from the regular header using the tabindex="-1" property when sticky value is true.

I am currently working on implementing this solution but facing some challenges but will try to quickly come with the MR. If anyone has an alternative approach or suggestions, I would greatly appreciate your input. Thank you!

🇮🇳India sandip

Hi @lavanyatalwar and @alok_singh, thanks for the work here. After reviewing the merge request, I believe there is no need for a media query for a specific screen width. The main issue seems to be related to box-sizing. Applying box-sizing: border-box; to the input tag should resolve the issue effectively.
Moving this issue to NW.

🇮🇳India sandip

Hi @utkarsh_kumar_singh, Thanks for the review but it seems a different issue and this issue only belongs to styling of the Read more and Add New Comment button only.
For that extra button it does not belong to this issue. Can you please reverify it once.

🇮🇳India sandip

Thanks @smustgrave! I'll definitely start looking into more advanced issues.

🇮🇳India sandip

If you see the codebase it is already there in the stable9/css/system/components/position-container.module.css. Please check once.

🇮🇳India sandip

Hi @saschaeggi, @smustgrave,

I have worked on fixing the issue and add some change to table.html.twig file as i applied a parent div to the table so i can apply overflow-x: auto to it as same as Gin. This issue is not only for Block layout page this issue will occur everywhere where table twig file is rendering so i had to change the table.html.twig file. As i see i also need to change the view-view-table.html.twig also as the same issue is coming from it. So should i change the views table twig file in this issue queue?

In the current MR, I haven't applied the changes for the Olivero theme. Please let me know if you would like me to include those adjustments as well.

Thank you!

🇮🇳India sandip

Another thing i want to ask that changes should be done for only Claro theme right? it does not make any sense to implement the same for Olivero as it is not a admin theme. Please correct me if i am wrong.

🇮🇳India sandip

I am working here to implement Gin's process as mentioned in #31.

🇮🇳India sandip

I think the MR is fine as the test failures are not related to this.
@rinku jacob 13, I think you deleted the before and after Screenshots by mistake. Can you Please Upload it again. So we can movie this issue to NR.

🇮🇳India sandip

Hi @kentr,
Thank you for the information. I'll proceed with raising a MR as outlined in the Proposed resolution.

I'm also only finding this one instance in Claro of an inline SVG. Maybe it would be better to convert it to a background image for consistency.

That sounds reasonable to me as well. If any maintainer agrees with this suggestion, I'll be happy to revert the changes. For now, I'll go ahead and raise the MR as mentioned in point #4.

🇮🇳India sandip

Hi @kentr, i think the proposed resolution may not work in this case as that hamberger menu is a svg image so we can add fill: linketext to its < path/> tag when forced color mode is active to fix this issue. Do you agree with it?

🇮🇳India sandip

I believe the current approach may not be optimal, as the issue specifically affects the Feed module. Fixing it in _layers.scss is causing unrelated changes elsewhere. To maintain consistency and prevent such issues, I will follow the standard used in the Gin theme by creating a dedicated feed.scss file inside the components folder for module-specific styling.

🇮🇳India sandip

My 2nd point was that in your previous commit bd6e4af7 after hovering over the div it is getting blue color as i thought you were trying to make the hover same as the other dropdowns like e.g. Security Advisory Coverage, Maintenance Status etc. I was saying if we follow this we need to change the color of the text from white to black when it get blue bg color while hovering.

Alternatively we may pick the design as i mentioned in my 3rd point it may look good as in light mode also Filter by category is not identical with other select dropdowns.

As you make the changes in the MR now. I think it is looking good. Let's wait for feedback from others.

🇮🇳India sandip

Removing the max-width property from the .page-wrapper class effectively resolves the issue. This adjustment eliminates the need for unnecessary max-width: 100% or any other values of max-width in the .page-wrapper class, ensuring the page will display full width.

🇮🇳India sandip

I have added a visible border to the active menu item in forced color mode. However, the task of increasing color contrast is still pending. As I’m currently occupied with other work, I’d appreciate it if someone could take it up. Thank you!

🇮🇳India sandip

Hi @utkarsh_33, While reviewing this issue, I had a few observations.

1. It seems that dark mode-related changes would be more appropriately placed in the gin.css file rather than pb.css, as all other Gin-
specific styling is handled there. Would you agree with this approach?

2. If we want it to look similar with the other select dropdowns we need to change the colour of text to black like other select dropdowns
when it gets focus or hover.

3. But i noticed in light mode this dropdown is not similar with other select dropdowns so we may consider it and can obtain the background color for before and after hover like this also.

I used this code for the changes

.gin--dark-mode .pb-filter__multi-dropdown__items--visible {
  background-color: var(--gin-bg-layer);
}
.gin--dark-mode .pb-filter__multi-dropdown__items--visible div:hover,
.gin--dark-mode .pb-filter__multi-dropdown__items--visible div:focus {
  background-color: var(--gin-bg-layer2);
}
🇮🇳India sandip

I am attaching a video for better understanding.

🇮🇳India sandip

Hi @saschaeggi, I have updated the changes for the Transition properties also. Can you please share your feedback on it.

🇮🇳India sandip

Hi @rkoller, @saschaeggi,

I am currently facing an issue where the second caret in the <select> element is coming from the browser’s default styling. When enabling forced color mode, the default arrow appears due to appearance: revert.

If I remove the background image, the browser’s default arrow remains, but the challenge is that it is positioned too far to the right, and we cannot style it. On the other hand, if I use appearance: none, the default arrow disappears, and we must rely on a background image, which we can style.

However, with prefers-color-scheme: dark and forced-color-mode: active, the background image needs to be dark, while for prefers-color-scheme: light with forced-color-mode: active, it should be white in colour.

Could you please advise on the best approach:

  1. Keeping the browser’s default arrow and removing the background image, or
  2. Removing the default arrow and using a background image that adapts based on prefers-color-scheme?

Looking forward to your suggestions.

🇮🇳India sandip

Go to Developer Tools on your browser, then ctrl+shift+P, then type force and click on the Emulate CSS forced-colors: active.
By this way you can active the forced colour mode on your browser.

You can see this resource: https://www.lambdatest.com/blog/windows-high-contrast-mode/

🇮🇳India sandip

Hi @sandeep_k,

Did you remember to clear your cache after applying the patch? Otherwise, the changes should have appeared on your local environment.

🇮🇳India sandip

Hi @danchadwick, I have done the changes can you please share your feedback on it.

🇮🇳India sandip

Hi @sandeep_k,

Previously, I worked on this using the Drupal CMS. I have now applied my merge request (MR) in Drupal 10.4.1, and the changes are clearly visible. Could you please try again from your side?

🇮🇳India sandip

Fixed some twerk as double carets were only appearing in dark mode so used unnecessary !important :( I had initially missed this, but now everything is looking good :)

🇮🇳India sandip

Please review the changes. I am attaching a video for reference.

🇮🇳India sandip

Thanks for the feedback now it is clear to me i am working on it.

🇮🇳India sandip

Yes @saschaeggi i was also thinking same. So i am applying the changes to the transition properties also. @rkoller and @mgifford can you please verify it once.

🇮🇳India sandip

@mgifford, Thanks for sharing the resource 😄

🇮🇳India sandip

Hi @rkoller, I raise a MR to use prefers-reduced-motion: no-preference for all animation properties in the codebase. Please review the changes. But i have one doubt should we do the same implementaion for transition property also?

🇮🇳India sandip

Thanks @mgifford for sharing the documentaions.
I am quickly trying to raise a MR for this issue with tested solution.

🇮🇳India sandip

Make the suggested changes :)

Before:

After:

🇮🇳India sandip

Hi @majmunbog,

While working on this issue, I noticed that the child menu items are not visible on hover. I also checked the "Show as expanded" option, but the nav-item--expanded class is still not being attached instead nav-item--collapsed is attaching with the

  • element. I verified this by dumping the classes variable in the menu.html.twig.

    Could you please share your feedback on this? Let me know if there's anything specific I should check or adjust before proceeding further.

    Looking forward to your guidance.

  • 🇮🇳India sandip

    Hi @tuuuukka, i have tried to reproduce this issue but i am not getting it. Can you enlighten me if i am missing something.
    Attached video for reference.

    🇮🇳India sandip

    Please review the changes. I think the test failures are not related to this issue so marking it as NR.

    🇮🇳India sandip

    Thank you @chrisfromredfin! Your appreciation means a lot to me. I’m still new to Svelte—I picked it up while working on this issue—but I’m excited to explore it further and apply it to future issues in Project Browser.
    Thanks again 😄

    🇮🇳India sandip

    Hi @rkoller and @simobm,

    I was reviewing this issue, and I believe it would be more appropriate to address it within the Gin Admin theme rather than making changes here. Could you please confirm if this approach makes sense, or if I might be overlooking something?

    🇮🇳India sandip

    Implementing theme settings to toggle the global dropdown behavior between hover and click.

    I am trying to implement this approach.

    🇮🇳India sandip

    Hi @utkarsh_33, I noticed that when MAX_SELECTIONS > 1, an unnecessary empty class attribute is being added to the . To address this, I propose using the following approach: <span class:pb-ellipsis={MAX_SELECTIONS === 1}>{message}</span>. This ensures that the class attribute is only included when MAX_SELECTIONS === 1, resulting in cleaner HTML output. Could you please share your thoughts on this solution?

    🇮🇳India sandip

    Increase the inset-inline-end more on `#project-browser .search__search-clear` as it was still overlapping with search icon when it gets focus.

    🇮🇳India sandip

    Hi @utkarsh_33, I am not getting any option in Project Browser UI to select multiple module to be installed (e.g. max_selections > 1). Can you guide me so i can take the SS and upload here too.

    🇮🇳India sandip

    Attaching before and after SS for better understanding.

    🇮🇳India sandip

    Please review the changes.

    🇮🇳India sandip

    Hi @omkar-pd,

    Thank you for your work on this. However, there’s still one more thing left to address. While you have removed the .pb-elipsis class from the CSS, we also need to remove it where it was attached to the install button. i am working on this.

    🇮🇳India sandip

    Hi @mudasirweb,

    I noticed that the image title is still positioned at the bottom. As I mentioned in #21, it should maintain integrity with the single image field. If I am mistaken, could someone kindly verify it?

    🇮🇳India sandip

    Hi @majmunbog,

    Thank you for your suggestion! I have implemented the utility class as you recommended to improve the styling approach. Could you please review the changes and share your feedback? Let me know if there are any further refinements needed.

    Production build 0.71.5 2024