Skip to content
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

Migrate NcModal to NcDialog #1239

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Jul 25, 2024

Fixes #1213

Reviewers: please test FileActionImport.vue dialog, because i was not able to see it in UI

Before After
Screenshot from 2024-07-25 18-42-30 Screenshot from 2024-07-25 18-46-24
Screenshot from 2024-07-25 18-43-03 Screenshot from 2024-07-25 18-46-04
Screenshot from 2024-07-25 18-43-56 Screenshot from 2024-07-25 19-11-04

Copy link
Contributor

@elzody elzody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being picky, just a few things

src/modules/modals/FileActionImport.vue Show resolved Hide resolved
src/modules/modals/FileActionImport.vue Show resolved Hide resolved
src/modules/modals/EditRow.vue Outdated Show resolved Hide resolved
src/modules/modals/EditColumn.vue Outdated Show resolved Hide resolved
src/modules/modals/CreateRow.vue Outdated Show resolved Hide resolved
@JuliaKirschenheuter JuliaKirschenheuter added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 26, 2024
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the bugfix-1213-Migrate-from-NcModal-to-NcDialog branch 2 times, most recently from adf2762 to 3bd4b7a Compare July 31, 2024 11:06
Copy link
Contributor

@elzody elzody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks wonderful, great fixes :)

@juliushaertl juliushaertl force-pushed the bugfix-1213-Migrate-from-NcModal-to-NcDialog branch from 3bd4b7a to 02fcb31 Compare August 2, 2024 13:47
@juliushaertl
Copy link
Member

Took the liberty to rebase and push a small adjustment for the cypress selectors that read the heading of the modals so we can get this in :)

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the bugfix-1213-Migrate-from-NcModal-to-NcDialog branch from 02fcb31 to 873fd22 Compare August 2, 2024 17:35
<NcModal v-if="showModal" size="large" @close="actionCancel">
<NcDialog v-if="showModal"
:name="t('tables', 'Create column')"
size="large"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also pass the buttons to the actions slot? That should make them sticky at the bottom of the modal so they are not hidden behind scroll bars

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@juliushaertl juliushaertl force-pushed the bugfix-1213-Migrate-from-NcModal-to-NcDialog branch from 873fd22 to e3602a0 Compare August 5, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from NcModal to NcDialog for consistent styles
3 participants