feat: Auto-focus on entity profile action modals #9264
Conversation
|
|
||
| // If the modal content is dynamic or the modal is not rendered immediately. | ||
| // we ned need to use the useEffect hook to ensure that the Select component gets the default value after the modal is mounted. | ||
| useEffect(() => { |
There was a problem hiding this comment.
nice! do we need to do this anywhere else? tags, glossary terms?
There was a problem hiding this comment.
yes where we want default value we have to do this
There was a problem hiding this comment.
in previous code we are using default value that's the reason I have added this code but in other place we are not using default value so not added this code there. In other places like glossary, term it is working expected without writing this code. If you want we can remove this code from EditWonerModal as well but in future we may required to add default value that's the reason I have kept this code here
| @@ -221,3 +221,6 @@ export function sortEntityProfileTabs(appConfig: AppConfig, entityType: EntityTy | |||
|
|
|||
| return sortedTabs; | |||
| } | |||
|
|
|||
| // To focus out from modal popup so autofocus works | |||
| export const getContainer = () => document.getElementById('root') as HTMLElement; | |||
There was a problem hiding this comment.
again, did we try to add the id to the app.tsx?
let's move this into a different place. let's put it in a root folder: src/app/utils/focus
There was a problem hiding this comment.
let's say getModalDomContainer.
Also please write a bit of a more detailed comment here.
There was a problem hiding this comment.
can we be 100% positive that the "root" id will never change?
There was a problem hiding this comment.
ReactDOM.render( <React.StrictMode> <App /> </React.StrictMode>, document.getElementById('root'), );
here also we are using root as id if this cannot be change we are 100% sure id will not be change
There was a problem hiding this comment.
let's move this into a different place. let's put it in a root folder: src/app/utils/focus.
sure will add this to app/utils/focus and change name to getModalDomContainer
There was a problem hiding this comment.
again, did we try to add the id to the app.tsx?
In App.tsx we have written all context and routing related code, for adding Id we have to add extra node of div and other person will be confused why we have given id there instead we can use root as id that will not change
There was a problem hiding this comment.
update the MR please review and let me know
| @@ -92,6 +93,7 @@ export default function EditTagTermsModal({ | |||
| defaultValues = [], | |||
| onOkOverride, | |||
| }: EditTagsModalProps) { | |||
| console.log(visible,"visible=====") | |||
| @@ -30,6 +30,15 @@ describe("glossary sidebar navigation test", () => { | |||
| cy.get('[data-testid="glossary-browser-sidebar"]').contains(glossaryTermGroup).click().wait(3000); | |||
| cy.get('*[class^="GlossaryEntitiesList"]').contains(glossaryTerm).should("be.visible"); | |||
|
|
|||
| // create glossaryParentGroup | |||
| cy.goToGlossaryList(); | |||
There was a problem hiding this comment.
Can you explain why this is necessary? Seems that glossary_navigation is still not passing : /
|
test failing so creating new branch |
Checklist