@sandip poddar

Account created on 20 September 2024, 6 months ago
#

Recent comments

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.

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.

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!

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);
}

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

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.

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/

Hi @sandeep_k,

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

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

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?

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 :)

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

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

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.

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?

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

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.

  • 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.

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

    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 😄

    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?

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

    I am trying to implement this approach.

    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?

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

    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.

    Attaching before and after SS for better understanding.

    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.

    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?

    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.

    I attempted to fix this issue by implementing Drupal.tabbingManager.constrain(element, { trapFocus: true });. However, after applying it, the tab focus gets trapped within the table, and I am unsure how to properly release it.

    As I have not been able to fully resolve the issue, I am unassigning myself for now so that others can contribute. If anyone has a solution or suggestions, please feel free to share them here. Your insights would be greatly appreciated!

    I have perfrom the suggested changes to all component templates. Please review the MR.

    Hi @sdhruvi5142, Can you please recheck on your side and dont forget to checkout to this branch

    3361765-footer-menus-overlapping

    . I again cloned the Fork repo on my local and checkout to the issue branch and get the suggested changes as usual.

    Hi @2dareis2do, I would be happy to work on your suggestions.

    Thinking about this I am not really sure why we need 5 footers as it is also possible and arrange 4 or 5 blocks in one or more regions. Maybe it would be better to simplify this and instead have types of footer?

    Do you see any issue with removing footers page.footer_second, page.footer_third and page.footer_fourth ?

    Based on your suggestion, should I consolidate these into a single region and allow blocks to be placed and arranged accordingly?

    For example, would something like the following be a better approach ??

    <div class="site-footer__top clearfix">
            {{ page.footer_first }}
     </div>

    Let me know your thoughts, and I’ll proceed accordingly. Thanks! 😊

    After rebase the Footer was looking like this below

    So i change the layout of Footer in its twig file and added styling according to it. Please review the changes :)

    The issue was caused by an inline display: block; style applied to the radio button input. I have resolved this by adjusting the styling accordingly. Please review the changes and let me know if any further modifications are needed.

    Hi @manishvijay02,

    Thank you for your work on this! I’ve reviewed your MR, and it successfully applies styles to the pagination as intended. To provide clarity, I’ve attached before and after screenshots for reference.

    I’m moving this issue to RTBC++.

    LGTM Thanks :)
    I am attaching before and after ss for clarification.
    Moving to RTBC++.

    For your reference you see this core issue queue https://www.drupal.org/project/drupal/issues/3456176#comment-15738200 🐛 10.3 upgrade now missing status-message theme sugestions Active

    Hi @utkarsh_kumar_singh,

    Thank you for your contribution! I appreciate your efforts on this. However, I have a few doubts that I’d like to clarify:

    In your MR, I noticed that you added custom JavaScript for closing the status message. This isn’t necessary, as applying the relevant patch should address the issue effectively.

    The main concern I observed is that the status message block isn’t rendering from the status-messages.html.twig file provided in the theme. This needs to be fixed. Could you please work on this?

    I’m updating the issue status to NW for now. Let me know if you need any assistance!

    Since there is no existing Gulpfile in the theme, I addressed this by leveraging the patch from an issue already in progress to create a Gulpfile. This issue has moved to RTBC as well, and I used its patch to convert SCSS to CSS.

    You can find more details about the issue here:
    https://www.drupal.org/project/school_theme/issues/3443060 📌 package.json and automation tools(gulpjs) are not in theme. Active

    HI @zetagraph,
    Thank you for identifying the issue with the size of the security icons—I appreciate your attention to detail. However, I believe this might not be directly related to the current issue. I belive we could keep this issue only for fixing the overlap issue of search icon and cross icon. Perhaps we could create a separate issue specifically for this?

    Please feel free to share your perspective and correct me if I’m mistaken as i am very new to contribution.

    HI @mudasirweb, Thanks for your work however, I have a couple of doubts that I would like to clarify

    1. Here below i attached ss for Single image field. So in case for multiple image field we should also maintain this design why we are putting
    the image title at the bottom.

    2. Could you please compile the SCSS file thoroughly? It seems to be causing a pipeline failure.

    Hi @manishvijay02,

    Thank you for your merge request! I’ve applied the changes, and it resolves the issue as shown in the attached screenshot for reference. However, I noticed that the .css file corresponding to the .scss changes is missing from the MR. Could you please push the related .css file to complete the update?

    Once that's done, it will be ready to be marked RTBC. For now, I’m moving it back to Needs Work.

    I think the drag button should be middle instead of top. I am working on it to fix.

    Oops, I didn’t realize this was related to an already resolved issue. It seems this is mostly fixed there. Unassigning myself from here

    Hi @akulsaxena,
    Thank you for your effort on the MR; I appreciate the work you’ve done so far! However, I have a couple of suggestions to make the implementation more modular and maintainable:

    1. Instead of writing the SCSS code for the footer in base.scss, it would be better to create a dedicated footer.scss file under the scss/components folder. This helps keep the styles more organized and modular.

    2. Regarding the use of #block-cascade-footer1, I noticed that this ID is dynamic and will change if the menu name changes (as shown in the attached screenshot). It would be more robust to use a static class or a specific selector tied to the block type to avoid future issues.

    3. It will be better if you follow this layout design for footer attached below

    Hi @akshay kashyap, i have updated as per your feedback on #6. Please share your review.

    I have added some padding in the search icon. Can you verify the alignment once.

    Hi @sourojeetpaul,
    I was previously working on this issue in Drupal 9, and I have now reproduced it in Drupal 10. Could you please review the MR From my side, the hover effect appears to be working as expected.

    Additionally, I resolved the merge conflict and made updates to menu--main.html.twig to address the issue.

    Hi @solideogloria, i think the version should be 1.x-dev instead of 2.x-dev. Can you please verify it once.

    Please review the changes. I have attached before and after Screenshots for reference.

    Apologies for the confusion earlier. I have now identified the issue, and your merge request resolves it effectively. I am attaching a screenshot of before and after for reference. I am moving this as RTBC++.

    I am not able to reproduce this issue in local.
    Drupal version: 10.4.1
    default admin theme: Gin

    Hi @anoopsingh92, after disabling Bigpipe module the issue is not coming. But instead of disabling Bigpipe module we can go with this solution

    use Drupal\Core\Render\Element\StatusMessages;
    
    function flexi_style_preprocess_block__system_messages_block(&$variables) {
      $variables['content'] = StatusMessages::renderMessages();
      $variables['#cache']['max-age'] = 0;
    }
    

    you can see this core issue for reference: https://www.drupal.org/project/drupal/issues/3456176#comment-15738200 🐛 10.3 upgrade now missing status-message theme sugestions Active . Kindly share your review on the changes.

    Hi @emanuelcoelho,
    I think we can check the authenticated user condition in function autologout_page_attachments_alter(array &$attachments) from where function autologout_attach_js(array &$element, array $settings) is called.
    If you agree i will start working on it.

    I have named the components as shown in the attached screenshot. If the naming isn't appropriate or could be improved, could you please suggest more standardized names?

    Does this work for you? Let me know if you'd like further adjustments!

    Production build 0.71.5 2024