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.

Sign up for GitHub

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

Jump to bottom

Close navigation on mobile on new form #380

Merged
merged 1 commit into from
May 22, 2020
Merged

Close navigation on mobile on new form #380

merged 1 commit into from
May 22, 2020

Conversation

jotoeri
Copy link
Member

@jotoeri jotoeri commented May 11, 2020

  • On mobile, the navigation doesn’t close when you click on a form or create new

@jotoeri jotoeri added bug Something isn't working 3. to review Waiting for reviews labels May 11, 2020
@jotoeri jotoeri added this to the 2.0 milestone May 11, 2020
@jotoeri jotoeri requested review from jancborchardt and skjnldsv May 11, 2020 19:52
@jotoeri
Copy link
Member Author

jotoeri commented May 11, 2020 •
edited
Loading

Ah - and edit: Ammended a very small fix, to center the emptyContent-Description-Text on mobile.
(Ok, different thing, but feels like too small for a separate PR)

@codecov-io
Copy link

Codecov Report

Merging #380 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #380   +/-   ##
========================================
  Coverage      0.00%   0.00%           
  Complexity      195     195           
========================================
  Files            17      17           
  Lines          1062    1062           
========================================
  Misses         1062    1062

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14213f9...170703a. Read the comment docs.

@jotoeri jotoeri mentioned this pull request May 11, 2020
Closed
20 tasks
jancborchardt
jancborchardt requested changes May 11, 2020
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Can confirm it closes on clicking "New form", but it still stays open when opening an existing form via the navigation, or e.g. also when going to responses of a form via the actions menu in the navigation.

@jotoeri
Copy link
Member Author

jotoeri commented May 12, 2020

Oups, seems like i read just half of the text.^^

@jotoeri jotoeri force-pushed the fix/mobile_nav branch 2 times, most recently from 3c9d295 to 7932ea1 Compare May 12, 2020 08:30
@jotoeri
Copy link
Member Author

jotoeri commented May 12, 2020

Still doesn't close on Click on Answers.

@skjnldsv :

  • Feels like the ActionRouter does not properly emit the @click event. Can you have a look onto this?
  • Maybe the whole process 'On mobile, close navigation on click' could be a property of ActionElements? Indeed, it's very specific, but might be very commonly useful...?

@jotoeri jotoeri added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 12, 2020
@jancborchardt
Copy link
Member

  • Maybe the whole process 'On mobile, close navigation on click' could be a property of ActionElements? Indeed, it's very specific, but might be very commonly useful...?

Might be good – if it’s possible to set per action. For example the navigation should be closed when clicking "Responses", but it should stay open when clicking "Copy share link" and "Delete form".

@jotoeri jotoeri added 3. to review Waiting for reviews pending fix Waiting for a fix on one of our dependencies and removed 2. developing Work in progress labels May 15, 2020
@jotoeri
Close navigation on mobile on new form
7494669 
Signed-off-by: Jonas Rittershofer <jotoeri@users.noreply.github.com>
@jotoeri
Copy link
Member Author

jotoeri commented May 17, 2020

Feels like the ActionRouter does not properly emit the @click event. Can you have a look onto this?

nextcloud-libraries/nextcloud-vue#1097

@jancborchardt jancborchardt mentioned this pull request May 22, 2020
Merged
@jancborchardt
Copy link
Member

nextcloud-libraries/nextcloud-vue#1097

That one is now merged 🎉

@jancborchardt jancborchardt self-requested a review May 22, 2020 16:30
jancborchardt
jancborchardt approved these changes May 22, 2020
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

👍

@jancborchardt jancborchardt merged commit 4a31509 into master May 22, 2020
@jancborchardt jancborchardt deleted the fix/mobile_nav branch May 22, 2020 16:30
@jotoeri
Copy link
Member Author

jotoeri commented May 22, 2020

Still needs a release, but yes 👍 :)

@Stanislav1975 Stanislav1975 mentioned this pull request Oct 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug Something isn't working feature: 📑 form creation pending fix Waiting for a fix on one of our dependencies
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jotoeri @codecov-io @jancborchardt