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

Add a continuous modal navigation flow for all page block #1465

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

kelanik8
Copy link
Contributor

@kelanik8 kelanik8 commented Oct 3, 2023

The following changes are implemented

TODO: Summary

Changes in the user interface:

TODO: Add screenshots, recordings or remove this section

Checklist when submitting a final (!draft) PR

  • Commits are tidied up, squashed if needed and follow guidelines in CONTRIBUTING.md
  • Code builds
  • All existing tests pass
  • All new critical code is covered by tests
  • PR is linked to the relevant issue(s)
  • Rebased with the target branch

@kelanik8 kelanik8 requested a review from Fajfa October 3, 2023 11:25
@kelanik8 kelanik8 self-assigned this Oct 3, 2023
@kelanik8 kelanik8 linked an issue Oct 3, 2023 that may be closed by this pull request
client/web/compose/src/components/Public/Record/Modal.vue Outdated Show resolved Hide resolved
@@ -126,10 +128,11 @@ export default {
clearRecordIDs: 'ui/clearRecordIDs',
}),

loadRecord ({ recordID, recordPageID, values, refRecord }) {
loadRecord ({ recordID, recordPageID, values, refRecord, editMode }) {
Copy link
Member

Choose a reason for hiding this comment

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

i would call all of these just edit, since it will make sense from the context what it is, same for this.modalInEditModal, can be called edit

client/web/compose/src/views/Public/Pages/Records/View.vue Outdated Show resolved Hide resolved
@katrinDY
Copy link
Contributor

Discuss with Jože if these changes should be added before starting to implement

What I've noticed:

  • after editing a record in a modal and closing it, the pb that displays the record isn't updated - RL, record organizer, calendar, geometry, chart. This can be addressed here
  • no 'x' btn in 'drill down' modal. Maybe add a bit of margin to modal title
Screenshot 2023-10-13 at 12 38 19 - no 'x' btn when chart is magnified(zoomed in)

@kelanik8 kelanik8 force-pushed the 2023.3.x-feature-proper-modal-navigation-flow branch from 4f6e15c to b446973 Compare October 17, 2023 12:37
@kelanik8 kelanik8 force-pushed the 2023.3.x-feature-proper-modal-navigation-flow branch from b446973 to fc37a73 Compare October 19, 2023 10:27
@kelanik8 kelanik8 changed the base branch from 2023.3.x to 2023.9.x October 19, 2023 10:28
@katrinDY
Copy link
Contributor

katrinDY commented Oct 20, 2023

One small thing: when mag. modal opens for a chart, the mag. icon is present first and then replaced with the x btn. Not sure if this is an issue. Discuss with Jože, please => update: this won't be addressed

3

@kelanik8 kelanik8 force-pushed the 2023.3.x-feature-proper-modal-navigation-flow branch from fc37a73 to eaa337c Compare October 24, 2023 13:19
@kelanik8 kelanik8 merged commit eaa337c into 2023.9.x Oct 26, 2023
1 check passed
@kelanik8 kelanik8 deleted the 2023.3.x-feature-proper-modal-navigation-flow branch October 26, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properly handle all navigation (record/page) inside modal flows
3 participants