-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add links to the minimap #246
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's looking good! Left 1 small suggestion :)
src/components/navigation/GlobalNavigation/GlobalNavigationItems.d.ts
Outdated
Show resolved
Hide resolved
src/components/navigation/GlobalNavigation/GlobalNavigation.stories.tsx
Outdated
Show resolved
Hide resolved
src/components/navigation/GlobalNavigation/GlobalNavigation.tsx
Outdated
Show resolved
Hide resolved
@@ -31,3 +32,5 @@ export interface IGlobalNavigationLink extends IBaseGlobalNavigationItem { | |||
} | |||
|
|||
export type IGlobalNavigationItem = IGlobalNavigationMenu | IGlobalNavigationLink | |||
|
|||
export type IMinimapOptions = { overviewHref: string; routes: ISvgLink[]; handleLinkClick: (route: string) => void } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question handleLinkClick
: by only looking at it, which link are we referring to? Is it all the routes? What about handleRoutesClick
?
Also, if those routes are predefined we should be able to type this route: string
better 🙏🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestions:
- change
handleLinkClick(string)
toonLinkClick(ISvgLink)
OR create aonClick
inside the each link (not sure what I would prefer) - rename
routes
tolinks
- break each field in one line. Not only to be consistent with the other types in this file, but it also makes new changes and reviews easier.
- use
interface
instead oftype
, because other types in this file are interfaces and you prefixed it withI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
break each field in one line. Not only to be consistent with the other types in this file, but it also makes new changes and reviews easier.
That's for prettier do decide. 😆 We have quite a large lineWidth configuration at 120, default was 80 but we tried to match better with current codebases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another breaking change for the api? why remove show
? indicative is already using in in an approved pr
im not against using the existence of minimapOptions
as a boolean, but seems like maybe the inteface wasnt fully thought out in the previous pr? also we should have a rule for this so its consistent, do we include a boolean show
or shouldShow
, or just short circuit on existence
a little similar to how navigationButtonItemOptions works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing it to remove the show
prop was my suggestion, totally agree on having a rule for this. I don't see the point of having a show property which also makes us have to declare all of the other properties, when the minimap in itself is optional. IMO short circuiting should be the way.
Yeah another breaking change :/ Minimap should not have been included in main but we're still figuring this whole trunk-based development thing. You're right interface wasn't fully thought out before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need a couple more changes in Indicative for reading from the UPS, new experience modal and popover reminder, so you could keep using the show
in the approved PR @jared-dickman and we can tackle this breaking change in the next update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's for prettier do decide. 😆 We have quite a large lineWidth configuration at 120, default was 80 but we tried to match better with current codebases.
(maybe the prettier rule would be singleAttributePerLine
and not lineWidth
, but I know how it's annoying with react comps with a few attrs)
Not sure if I agree this is up to prettier. It's not a stylish detail, I see some good technical reasons to break it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So @gabyzif added a new property to the IMinimapOptions
and that seems to have solved the problem cause now Prettier is breaking the lines. But let me give some thoughts on formatting.
I know how it's annoying with react comps with a few attrs)
I actually don't find it annoying. For me, more than 2 properties is already enough for breaking, but that's personal opinion. Prettier doesn't allow this to be configurable, so that's what I meant with it's up for Prettier to decide. It means if we are using Prettier, we don't have a choice here 😅 (Other than disabling it and stop using it or going with another formatting tool)
If we had something more close to the default lineWidth
, this would be solved, but we decided for 120
when we created the rules for the Aquarium so we sometimes end up with those long lines, as we would end up with sometimes too much new lines if we had the default at 80
.
It's harsh, and opinionated, but that's a good thing IMO cause we avoid this types of discussions on formatting.
I see some good technical reasons to break it
Can you elaborate? Maybe you're thinking of readability and maintainability improvements? But that doesn't change anything technical about the code.
Either way @gabrielmarquesf let me know if the thoughts on formatting make sense, it's a great topic for discussion now that we're looking into linting/formatting for Nancy 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poor @gabyzif we love to use your PR's for discussion 😆
Just ignore the above in regards to the PR review please 🙏🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you are right, by "technical reasons" I meant readability and maintainability. Besides the well known benefits, I raise how this makes historical changes tracking/finding easier, and it's very important imo.
And yes, makes sense to me everything you mentioned! ;)
export interface ISvgLink { | ||
elementId: string | ||
route: string | ||
isAuthorized: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabrielmarquesf Do you think we will ever need a 3rd status here? Like isAuthorized
is currently boolean
, but could we ever need something like permission: 'authorized' | 'not-authorized' | 'view-only' | 'blocked'
(only examples).
Trying to think if it makes sense to get rid of the boolean and have type instead :)
I tend to be against those isSomething
type of props cause we always end up needing more statuses and then we end up with a thousand boolean props.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I see what you say, and makes sense... but I could also say that don't like to pre-engineer and keep it simple 😃 I left another suggestion for this above, it's up to @gabyzif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hahaha I get it. Can be pre-engineering for sure. Problem is, if we ever decide to change we need to introduce a breaking change :/
So if there's only the slight possibility of having multiple status I would still go with an union string type.
src/components/navigation/GlobalNavigation/GlobalNavigation.tsx
Outdated
Show resolved
Hide resolved
@@ -31,3 +32,5 @@ export interface IGlobalNavigationLink extends IBaseGlobalNavigationItem { | |||
} | |||
|
|||
export type IGlobalNavigationItem = IGlobalNavigationMenu | IGlobalNavigationLink | |||
|
|||
export type IMinimapOptions = { overviewHref: string; routes: ISvgLink[]; handleLinkClick: (route: string) => void } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestions:
- change
handleLinkClick(string)
toonLinkClick(ISvgLink)
OR create aonClick
inside the each link (not sure what I would prefer) - rename
routes
tolinks
- break each field in one line. Not only to be consistent with the other types in this file, but it also makes new changes and reviews easier.
- use
interface
instead oftype
, because other types in this file are interfaces and you prefixed it withI
@@ -41,9 +46,18 @@ const TooltipWithButton: React.FC<TooltipWithButtonProps> = ({ onClick }) => ( | |||
) | |||
|
|||
export const HomeButton: React.FC<HomeButtonProps> = props => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with aquarium codebase, but I don't see other comps using this React.FC<...>
or deconstructing props (in Nancy I believe we never used it). I would suggest to change it to be consistent, but this might be a good time to suggest this pattern to the team if you feel strong about it.
const MpHomeButton: React.FC<MpHomeButtonProps> = ({ onClick }) => ( | ||
<Center className="globalNavigation__mpHome" onClick={onClick}> | ||
<Icon name="mpLogo" size="lg" color="white" /> | ||
</Center> | ||
) | ||
|
||
const MinimapWithPopover: React.FC<MinimapWithPopoverProps> = ({ overviewHref, onClick }) => ( | ||
<Popover content={() => <MiniMap overviewHref={overviewHref} />} placement="rightBottom" arrow={false}> | ||
const MinimapWithPopover: React.FC<MinimapWithPopoverProps> = ({ overviewHref, onClick, routes, handleLinkClick }) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per our styleguide, we should not descructure props but rather introduce a named interface
and a reminder that this take ~1 second and ~0 typing with some clever ide shortcuts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kudos for bringing our styleguide. Although it should be an automated, we gotta stick to manually doing it while we're not there yet.
<MinimapWithPopover overviewHref={props.minimapOptions?.overviewHref ?? '/'} onClick={props.onMpHomeClick} /> | ||
) : ( | ||
<TooltipWithButton onClick={props.onMpHomeClick} /> | ||
const { minimapOptions, onMpHomeClick } = props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just me here but im not a fan of destructuring props, as i find it informative to see props.thing
when reading through the component implementation, knowing if something came from props or a local var/state can be insightful
} | ||
|
||
const Minimap: React.FC<IMinimapProps> = props => { | ||
const Minimap: React.FC<IMinimapProps> = ({ overviewHref, routes, handleLinkClick }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const Minimap: React.FC<IMinimapProps> = ({ overviewHref, routes, handleLinkClick }) => { | |
const Minimap: React.FC<IMinimapProps> = (props: IMinimapProps) => { |
|
||
const buttonsWithRoutes: ISvgLink[] = routes.map(route => ({ | ||
elementId: linkMap[route.elementId] || route.elementId, | ||
route: route.route, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yo dawg i heard you like routes
[buttons, onLinkClick], | ||
) | ||
|
||
const wrapButtonsIntoLinks = (parent: React.ReactNode): React.ReactNode => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code was duplicated from nancy yea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup. the idea at some point is that we use the linker in nancy too :) for the overview map
} | ||
|
||
&.svg-linker-root__button--drop-shadow { | ||
& > g > g > rect { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should consider changing these selectors to be more explicit. naming the parts of the svg we want to select [perhaps event with an id? though a class should work right?]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to sync it with UX, since they are exporting those SVGs for us. I will handle it!, and I think we can go ahead with this PR meanwhile.
@@ -1,3 +1,62 @@ | |||
.minimap_container{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats with this class name?!
Co-authored-by: jared-dickman <zenador15@gmail.com>
## [1.15.1-minimap-linker.1](v1.15.0...v1.15.1-minimap-linker.1) (2024-05-30)
href: string | ||
} | ||
|
||
export interface IMinimapOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export interface IMinimapOptions { | |
export interface IMiniMapOptions { |
elementId: string | ||
href: string | ||
variant?: 'regular' | 'black' | 'drop-shadow' | ||
isUnAuthorized?: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isUnAuthorized?: boolean | |
isUnauthorized?: boolean |
.minimap_container { | ||
padding: var(--padding-sm); | ||
} No newline at end of file | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't work to replace it by a util class?
} | ||
|
||
&.svg-linker-root__button--drop-shadow { | ||
& > g > g > rect { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to sync it with UX, since they are exporting those SVGs for us. I will handle it!, and I think we can go ahead with this PR meanwhile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job!
# [1.16.0](v1.15.0...v1.16.0) (2024-05-30) ### Features * Add links to the minimap ([#246](#246)) ([80bd00c](80bd00c))
🎉 This PR is included in version 1.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# 1.0.0-fix-semantic-release.1 (2024-10-28) ### Bug Fixes * Design tokens update + icons alignment fix ([#460](#460)) ([9c93165](9c93165)) * a useEffect Hook has been added to detect if the value received … ([#339](#339)) ([0e1ab5d](0e1ab5d)) * active workspace style ([#148](#148)) ([a276806](a276806)) * add current color fill to icon ([#374](#374)) ([12c698c](12c698c)) * add horizontal gap in suite selector ([#391](#391)) ([74a43d0](74a43d0)) * add icons ([#399](#399)) ([3343c26](3343c26)) * add icons for overview map ([#275](#275)) ([a4bb97d](a4bb97d)) * Add Icons in Aquarium ([#395](#395)) ([d554ed5](d554ed5)) * add info icon ([#381](#381)) ([d3aa957](d3aa957)) * add more icon ([#403](#403)) ([3abc3b7](3abc3b7)) * add new icons ([#380](#380)) ([db88c4e](db88c4e)) * add width auto to be able to manage the wdith from the component ([#368](#368)) ([b47c45f](b47c45f)) * button-alligned center ([#287](#287)) ([d8c056d](d8c056d)) * **cascader:** makes cascader use the aquarium configprovider and theme ([21800e2](21800e2)) * correct QueryItem Cascader and Action cosmetic bugs/flaws ([f5e6032](f5e6032)) * correct usage of Typography in Watermark.stories.tsx ([f8b32cb](f8b32cb)) * ensure PostCSS handles CSS unnesting in Vite config ([#457](#457)) ([1978e65](1978e65)) * export notification center props ([#418](#418)) ([2ea78c2](2ea78c2)) * exported types ([#448](#448)) ([a978175](a978175)) * fix broken stories due to typography update ([d64be86](d64be86)) * fix issue on prettifier ([#446](#446)) ([03668b6](03668b6)) * fix QueryItem.ValueSelector.Cascader's selectedDisplayValue type ([e74053d](e74053d)) * fixes path to be / on UPS cookie creation options ([#240](#240)) ([0cebf44](0cebf44)) * hide expand icon in global navigation lists ([#178](#178)) ([edd21db](edd21db)) * hide minimap after user clicks on a button ([#264](#264)) ([21a399c](21a399c)) * icon color not changing on button hover ([#363](#363)) ([aa07067](aa07067)) * Icon component build issue ([d7e2faf](d7e2faf)) * improve workspace selector performace ([#370](#370)) ([590e231](590e231)) * Input now support refs ([#169](#169)) ([20a7e47](20a7e47)) * **input:** wraps Input.Search in Aquarium ConfigProvider ([0fbba18](0fbba18)) * lazy load ValueSelector to fix Storybook test runner ([#298](#298)) ([4cd501b](4cd501b)) * **lint:** fix linting problems ([c343136](c343136)) * make onChange prop of QueryItem InputText optional ([7a1532f](7a1532f)) * make QueryItem Cascader options reactive ([50f11bc](50f11bc)) * make the workspace-selector label smaller ([#359](#359)) ([05c4338](05c4338)) * minor compatibility changes to QueryItem Qualifier and Cascader ([5065bdd](5065bdd)) * minor tweaks to TextInput to make it fully compatible ([be5deab](be5deab)) * nav link target ([#166](#166)) ([092a3aa](092a3aa)) * **NavigationSearch:** swaps Ctrl for Cmd on Macs ([a0b7d63](a0b7d63)) * notification center export z-index ([#422](#422)) ([ce01227](ce01227)) * PR review suggestions ([f1ed504](f1ed504)) * **queryitem:** change Input.Search to Input in Cascader's popover to prevent preact type error ([80b1469](80b1469)) * remove -t patch from sem release ([fa41baa](fa41baa)) * remove \dist files accidentally committed ([ef5a16b](ef5a16b)) * remove circle-dashed.svg, it's a duplicate ([aac4829](aac4829)) * remove hover effect on active state ([#278](#278)) ([9f89fa1](9f89fa1)) * remove LESS syntax from query-item.css ([643402c](643402c)) * rename icons ([#364](#364)) ([223c47d](223c47d)) * Revert design tokens update ([#361](#361)) ([cd9dbc9](cd9dbc9)) * rollback variables/style ([#310](#310)) ([efcaf5c](efcaf5c)) * signout onclick being triggered on render when there are no orgs ([#337](#337)) ([77f2b1a](77f2b1a)) * style build missing custom rules ([#147](#147)) ([3a66b26](3a66b26)) * sub menu links should open the correct herf ([#167](#167)) ([6b52fea](6b52fea)) * **typography:** fix exporting/structure of Typography and its subtypes ([cd097f8](cd097f8)) * unnest css ([#390](#390)) ([03d3ccb](03d3ccb)) * unnesting minimap styles ([#253](#253)) ([067f24d](067f24d)) * update design tokens ([#375](#375)) ([e818591](e818591)) * update empty component ([#371](#371)) ([655e252](655e252)) * update icons that did not obey the color prop ([#445](#445)) ([68006e3](68006e3)) * update more typography stories ([cd3344c](cd3344c)) * update nav link active background color ([#376](#376)) ([3965e60](3965e60)) * update release.config.cjs ([859b3bb](859b3bb)) * update release.config.cjs tag format version ([93ee2ec](93ee2ec)) * update release.yml to test semantic release ([5dcf0a7](5dcf0a7)) * use semantic release token ([7af705c](7af705c)) * use unique tags ([61d7886](61d7886)) * workspace selector positioning ([#338](#338)) ([9561a49](9561a49)) * **workspaceselector:** fix key duplication in the WorkspaceSelector ([2967c68](2967c68)) * **workspaceselector:** fix performance problems by swapping Antd Menu component with native ul ([8a0257b](8a0257b)) * yet more typography corrections ([81fce66](81fce66)) ### Features * add Action QueryItem ([f76ea2b](f76ea2b)) * add datepicker example ([#408](#408)) ([0b4b46b](0b4b46b)) * Add directory icon ([#319](#319)) ([a2b4132](a2b4132)) * add Help Icon ([6bae0ed](6bae0ed)) * Add links to the minimap ([#246](#246)) ([80bd00c](80bd00c)) * add loadData functionality to QueryItem Cascader ([ced4994](ced4994)) * add lock/unlock icons; rename previous lock to paywall ([#340](#340)) ([eaf6998](eaf6998)) * add mp colors to storybook and remove token to css from pre com… ([#405](#405)) ([40dffbf](40dffbf)) * add notification center ([#416](#416)) ([4c44206](4c44206)) * Add notification icon ([#345](#345)) ([2c9d13e](2c9d13e)) * add overview dt icon ([#362](#362)) ([78d7854](78d7854)) * add placeholder to QueryItem's TextInput ([30bde49](30bde49)) * add premiumDt icon ([#346](#346)) ([b3e2817](b3e2817)) * add prettifier ([#404](#404)) ([a99793f](a99793f)) * add QueryItem-related icons ([89fb3b8](89fb3b8)) * add QueryItem.Text component ([8f0b7bd](8f0b7bd)) * Add size prop to `Typography.Text` component ([#262](#262)) ([0c14f00](0c14f00)) * add value props to Qualifier and Cascader ([16042d7](16042d7)) * add variants to icons ([#355](#355)) ([1112674](1112674)) * add workspace label to Workspace Selector ([#323](#323)) ([bdb3c70](bdb3c70)) * Adds stylelint with recommended configs and rules from Indicative ([#82](#82)) ([4dd5380](4dd5380)) * Adds SuitesReminder hook to get a consistent look across platforms for the reminder notification ([#221](#221)) ([533428e](533428e)) * Allow importing types ([#65](#65)) ([7fe8981](7fe8981)) * Allow override theme for ConfigProvider ([#354](#354)) ([29cc614](29cc614)) * Allows publishing npm versions from feature branches ([#186](#186)) ([044c117](044c117)) * Allows publishing on dev branch/distribution channel ([#162](#162)) ([39530fa](39530fa)) * allows UPS to receive cookie config options ([#261](#261)) ([8ab9ce9](8ab9ce9)) * ant design system ([#9](#9)) ([7c0f090](7c0f090)) * ant upgrade to 5.20 ([#421](#421)) ([49b4bf8](49b4bf8)) * components ([#33](#33)) ([78dfb17](78dfb17)) * Exports bundle without libs (React, Antd) and TS types ([#76](#76)) ([5dbb9df](5dbb9df)) * first story with a simple component and directory structure with readmes ([a60b9b8](a60b9b8)) * focus workspace on icon click ([#163](#163)) ([cea91cd](cea91cd)) * global navigation and styling ([#124](#124)) ([7863d9a](7863d9a)) * icons placeholder ([#385](#385)) ([2503af1](2503af1)) * **icons:** export icons from the Aquarium ([dde4db2](dde4db2)) * install vitest unit test runner ([#143](#143)) ([2ad4075](2ad4075)) * intial add of QueryItem.ValueSelector.Cascader ([c022aa2](c022aa2)) * minimap active state ([#263](#263)) ([ae9a2b0](ae9a2b0)) * move minimap to suite logo ([#369](#369)) ([f2d2437](f2d2437)) * new icons ([#239](#239)) ([a1bfb88](a1bfb88)) * New storybook documentation structure ([#419](#419)) ([1a870bc](1a870bc)) * Organize component directories by type ([#16](#16)) ([6ad046d](6ad046d)) * query select number ([#268](#268)) ([13dc864](13dc864)) * **queryitem:** adding onchange and converting error danger zone text ([91e945f](91e945f)) * **queryitem:** adding QueryItem.Qualifier component ([1fe60d8](1fe60d8)) * **queryitem:** updating styles and adding error message to qualifier ([fa8a8da](fa8a8da)) * replace minimap with suiteSelector component ([#383](#383)) ([2ab27ca](2ab27ca)) * rework Cascader OnChange ([a77c44f](a77c44f)) * Storybook Testing ([#42](#42)) ([876c231](876c231)) * Tag component stories ([#68](#68)) ([4831b25](4831b25)) * update design tokens ([#273](#273)) ([ac2b442](ac2b442)) * update SuiteLogo in GlobalNav to render nav switcher Tour ([#367](#367)) ([a3920af](a3920af)) * UPS code ported from Nancy ([#207](#207)) ([cc153af](cc153af)) * wrap radio group and button components ([#282](#282)) ([6e67439](6e67439))
This PR adds an SVG linker (like the one we have on MP for the overview map). Design gave us an SVGs with IDs for the minimap and with this component we are making the boxes clickable and with hover effects)
Note: I tried importing the SVG from a file like we do with the icons but its not working because the children IDs from the are not being passed. I'm going to investigate it in a TD task.
Note: this PR is under development.