[KED-2633] Add split panel component and implement into sidebar #448
Conversation
limdauto
force-pushed
the
feature/split-panel
branch
from
aeeffe8 to
6eb3c1f
Compare
package/requirements.txt
Outdated
| @@ -1,4 +1,4 @@ | |||
| semver~=2.10 # Needs to be at least 2.10.0 to get VersionInfo.match | |||
| Flask>=1.0, <2.0 | |||
| Flask>=2.0 | |||
There was a problem hiding this comment.
Is this change related to / required by this PR?
There was a problem hiding this comment.
Just saw the commit history and it looks like this has been updated yesterday from lim - perhaps this should be introduced in a seperate PR?
There was a problem hiding this comment.
Yeah this was a temporary fix by @limdauto, I think this is resolved in the main branch now though which I've merged here.
src/components/split-panel/index.js
Outdated
| handle: handleRef.current?.getBoundingClientRect(), | ||
| }); | ||
|
|
||
| const clampSplit = (value) => { |
There was a problem hiding this comment.
I know it's hard to find a good variable name for this, but clampSplit sounds a bit confusing - perhaps calling it 'calculateSplitorcalculateXX` ( whatever it is trying to calculate) would be more intuitive?
There was a problem hiding this comment.
Fair enough if clamp seems to clash with split I guess, I've refactored this to just clamp then as calculate doesn't really provide any information on what it does.
src/components/split-panel/index.js
Outdated
| * @param {?number} [splitMax=1] A number [0...1] as the maximum % split position | ||
| * @param {?number} [keyboardStep=0.025] A number [0...1] as the % step to move split when using keyboard | ||
| * @param {?string} [orientation='vertical'] Only 'vertical' currently supported. | ||
| * @return {object} The rendered children |
There was a problem hiding this comment.
It would be more clear to add some example of what the children usually is and any expected format of the children in the comment, such as the react component it is encapsulating, etc
There was a problem hiding this comment.
Sure I've improved the documentation in this file, including a note that the test file serves as a reference usage.
There was a problem hiding this comment.
Really great work @bru5 , tested across browsers and also on the lib-test, works well in all cases. Well done 👍
…ro-viz into feature/split-panel
# Conflicts: # package/requirements.txt
Description
Adds a new split panel component and splits the current sidebar into two sections.
Development notes
There is a new generic
SplitPanelcomponent, which implements user resizing.The existing sidebar has been split into two panels, the elements section is now in the top and the categories are in the bottom panel.
This work is the first part of a series of design changes coming to the sidebar.
QA notes
Other than the split panel, functionality is the same as existing, including all existing search and filters which should be tested through and compared to the previous release.
Usability should also be tested, focus and keyboard arrows are implemented.
Checklist
RELEASE.mdfileLegal notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.