Update the style of the square buttons and add border radius in all buttons, inputs, selectors #11425
Conversation
| export const markEdgesForToolbar = (el) => { | ||
| if (!el) return; | ||
| const buttons = Array.from(el.querySelectorAll('button')); | ||
| // filter out excluded + invisible | ||
| const eligible = buttons.filter((btn) => isVisible(btn)); | ||
| // remove old markers | ||
| buttons.forEach((btn) => btn.classList.remove('is-first', 'is-last')); | ||
| // mark new first/last | ||
| if (eligible.length > 0) { | ||
| eligible[0].classList.add('is-first'); | ||
| eligible[eligible.length - 1].classList.add('is-last'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
We should not manipulate directly classes in the DOM, in general we should give priority on react or css solutions and as last resource access to the DOM trying to reduce at minimum the manipulation.
Here my suggestion is to only find which are the first and last indices of the visible button and then inside the React component use useState to store and add the classes to the div wrapper
export const getEdgesIndexForToolbar = (el) => {
if (!el) return [];
const eligible = [...(el.children || [])].map((child, idx) => [idx, child]).filter((entry) => entry[1].children.length > 0 && entry[1].style.visibility !== 'hidden');
const [firstIndex] = eligible[0] || [];
const [lastIndex] = eligible[eligible.length - 1] || [];
return [firstIndex, lastIndex];
};There was a problem hiding this comment.
Updated the function to use the above implementation instead of using the DOM manipulation.
| const width = ref.current.clientWidth; | ||
| const scrollWidth = ref.current.scrollWidth; | ||
| const overflow = width < scrollWidth; | ||
| markEdgesForToolbar(ref.current); |
There was a problem hiding this comment.
Here we could add a internal useState:
setEdgesIndex(getEdgesIndexForToolbar(ref.current));
and then apply the class to the div wrapper only if the idx match the first and last visible not empty item
There was a problem hiding this comment.
Used the new function here to set the edges button index and used the index to set the class in the component.
|
@subashtiwari1010 I started to review the PR but not completed it yet. You can start to address the changes requested in this initial review, thanks |
tdipisa
added
the
BackportNeeded
tdipisa
removed
the
BackportNeeded
4 participants
Description
This PR updates the style of the square buttons, and adds the border radius in button, inputs and selectors as per the requirement in #10954.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
What is the current behavior?
The buttons were categorized in small, medium and default size. And the components did not have the border radius to them.
Fixes #10954
What is the new behavior?
All the buttons are now categorized under
square-buttononly. Other categories of square-button-md and square-button-sm are deprecated. Also, the border radius have been implemented in the button, inputs and select components.Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information