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

Update window title #368

Merged
merged 1 commit into from
May 9, 2020
Merged

Update window title #368

merged 1 commit into from
May 9, 2020

Conversation

jotoeri
Copy link
Member

@jotoeri jotoeri commented May 9, 2020

  • The title is just "Forms - Nextcloud" but should be "Form name - Forms - Nextcloud" so you can see the relevant part even when the tab is short. Same for share view

@jotoeri jotoeri added bug Something isn't working 3. to review Waiting for reviews labels May 9, 2020
@jotoeri jotoeri added this to the 2.0 milestone May 9, 2020
@jotoeri jotoeri requested review from jancborchardt and skjnldsv May 9, 2020 13:42
@jotoeri jotoeri mentioned this pull request May 9, 2020
Closed
20 tasks
jancborchardt
jancborchardt approved these changes May 9, 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.

Great stuff, works nicely! :)

@jotoeri
Fix window title
3d3bb97 
Signed-off-by: Jonas Rittershofer <jotoeri@users.noreply.github.com>
@jotoeri jotoeri force-pushed the fix/window_title branch from 78f99e4 to 3d3bb97 Compare May 9, 2020 15:37
@jancborchardt jancborchardt merged commit 8d1cd28 into master May 9, 2020
@jancborchardt jancborchardt deleted the fix/window_title branch May 9, 2020 16:37
skjnldsv
skjnldsv reviewed May 9, 2020
View reviewed changes
src/mixins/WindowTitleMixin.js

export default {
methods: {
setWindowTitle(formTitle) {
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer this as a function to import, rather than a mixin. Because it really doesn't need the full component context :)
@jotoeri

Copy link
Member Author

Choose a reason for hiding this comment

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

:D shit. I see, i should wait for two approvals even on small changes. 🤦‍♂️
I'll change it.

Copy link
Member

Choose a reason for hiding this comment

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

Usually we do aim for 2 in nextcloud, but there isn't too much devs here (3), so after release we could bump it to 2 to make sure ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv left review comments

@jancborchardt jancborchardt jancborchardt approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jotoeri @jancborchardt @skjnldsv