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

feat: add NcAppNavigationSettingsButton #5866

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat: add NcAppNavigationSettingsButton #5866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3382a4a
chore(NcAppNavigationSettings): deprecate component
ShGKme Jul 23, 2024
10eae39
refactor(NcAppNavigationSettings): remove $app-navigation-settings-ma…
ShGKme Jul 24, 2024
775795f
feat(NcAppNavigationSettingsButton): add settings button for App Navi…
ShGKme Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/assets/variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,5 @@ $navigation-width: 300px;
$breakpoint-mobile: 1024px;
$breakpoint-small-mobile: math.div($breakpoint-mobile, 2);

// navigation spacing
$app-navigation-settings-margin: 3px;

// navigation items
$app-navigation-padding: calc(var(--default-grid-baseline, 4px) * 2);
43 changes: 15 additions & 28 deletions src/components/NcAppNavigation/NcAppNavigation.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
<NcAppNavigationSearch v-model="searchValue">
<template #actions>
<NcActions>
<NcActionButton close-after-click @click="showModal = true">
<NcActionButton close-after-click @click="isSettingsDialogOpen = true">
<template #icon>
<IconCog :size="20" />
</template>
App settings (close after click)
</NcActionButton>
<NcActionButton @click="showModal = true">
<NcActionButton @click="isSettingsDialogOpen = true">
<template #icon>
<IconCog :size="20" />
</template>
Expand All @@ -36,28 +36,20 @@
</NcAppNavigationItem>
</template>
<template #footer>
<div class="navigation__footer">
<NcButton wide @click="showModal = true">
<template #icon>
<IconCog />
</template>
App settings
</NcButton>
<NcModal v-if="showModal" name="Modal for focus-trap check" @close="showModal = false">
<div class="modal-content">
<h4>Focus-trap should be locked inside the modal</h4>
<NcTextField :value.sync="modalValue" label="Focus me" />
</div>
</NcModal>
</div>
<NcAppNavigationSettingsButton name="Example settings" @click="isSettingsDialogOpen = true" />
<NcAppSettingsDialog :open.sync="isSettingsDialogOpen" name="Example settings">
<NcAppSettingsSection id="section-1" name="Section 1">
<NcTextField label="Setting field" :value.sync="modelValue" />
</NcAppSettingsSection>
</NcAppSettingsDialog>
</template>
</NcAppNavigation>
</div>
</template>

<script>
import IconCheck from 'vue-material-design-icons/Check'
import IconCog from 'vue-material-design-icons/Cog'
import IconCheck from 'vue-material-design-icons/Check.vue'
import IconCog from 'vue-material-design-icons/Cog.vue'

export default {
components: {
Expand All @@ -71,10 +63,10 @@
},
data() {
return {
items: Array.from({ length: 5 }, (v, i) => `Item ${i+1}`),
items: Array.from({ length: 10 }, (v, i) => `Item ${i + 1}`),
searchValue: '',
modalValue: '',
showModal: false,
modelValue: '',
isSettingsDialogOpen: false,
}
},
}
Expand All @@ -84,7 +76,7 @@
/* Mock NcContent */
.styleguide-nc-content {
position: relative;
height: 300px;
height: 290px;
background-color: var(--color-background-plain);
overflow: hidden;
}
Expand All @@ -96,11 +88,6 @@
gap: 4px;
padding: 4px;
}

.modal-content {
height: 120px;
padding: 10px;
}
</style>
```

Expand Down Expand Up @@ -160,7 +147,7 @@ emit('toggle-navigation', {
<slot name="list" />
</NcAppNavigationList>

<!-- @slot Footer for e.g. NcAppNavigationSettings -->
<!-- @slot Footer for e.g. NcAppNavigationSettingsButton and NcAppSettingsDialog -->
<slot name="footer" />
</nav>
<NcAppNavigationToggle :open="open" @update:open="toggleNavigation" />
Expand Down
12 changes: 12 additions & 0 deletions src/components/NcAppNavigationSettings/NcAppNavigationSettings.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
- SPDX-License-Identifier: AGPL-3.0-or-later
-->

<docs>
⚠️ This component is deprecated and will be removed in v9.

Use `<NcAppNavigationSettingsButton>` with `<NcAppSettingsButton>` instead.
Copy link
Contributor

@susnux susnux Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

NcAppSettingsButton ? You mean NcAppSettingsDialog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

</docs>

<template>
<div id="app-settings"
v-click-outside="clickOutsideConfig"
Expand All @@ -26,6 +32,10 @@
</template>

<script>
/**
* @deprecated This component is deprecated and will be removed in v9. Use NcAppNavigationSettingsButton + NcAppSettingsButton instead.
*/

import { t } from '../../l10n.js'
import { clickOutsideOptions } from '../../mixins/index.js'

Expand Down Expand Up @@ -74,6 +84,8 @@ export default {
}
</script>
<style lang="scss" scoped>
$app-navigation-settings-margin: 3px;

#app-settings {
margin-top: auto;
padding: $app-navigation-settings-margin;
Expand Down
55 changes: 55 additions & 0 deletions src/components/NcAppNavigationSettingsButton/NcAppNavigationSettingsButton.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!--
- SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: AGPL-3.0-or-later
-->

<docs>
The component to be used as a settings button in the `<NcAppNavigation>` to open `<NcAppSettingsDialog>`.
</docs>

<template>
<div class="app-navigation-settings-button">
<NcButton class="app-navigation-settings-button__button"
type="tertiary"
alignment="start"
wide
@click.stop="emit('click', $event)">
<template #icon>
<IconCog :size="20" />
</template>
<span class="app-navigation-settings-button__text">
{{ name }}
</span>
</NcButton>
</div>
</template>

<script setup>
import { t } from '../../l10n.js'
import NcButton from '../NcButton/index.js'
import IconCog from 'vue-material-design-icons/Cog.vue'

defineProps({
/**
* The name of the settings button. For example, "Files Settings" or "Talk Settings".
*/
name: {
type: String,
required: false,
default: t('Settings'),
},
})

const emit = defineEmits(['click'])
</script>

<style lang="scss" scoped>
.app-navigation-settings-button {
padding: var(--default-grid-baseline);

&__text {
// Make the settings button less presentative, like app navigation items
font-weight: normal;
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels every different to all other elements and is different to all current implementations, I would like to ask @nextcloud-libraries/designers first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels every different to all other elements and is different to all current implementations

I actually agree, and that's exactly what we were discussing yesterday.

But from yesterday's design review call:

  • It should not be bold or secondary, otherwise it is too presentive, more than other items in the menu.
  • It should be the same in all navigation menus, no matter if NcAppNavigationItem or NcListItem is used
  • It should be aligned with navigation items (NcAppNavigationItem)

cc @jancborchardt

Copy link
Contributor

Choose a reason for hiding this comment

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

To confirm – what @ShGKme listed is what I said the requirements are. The design in this PR fulfills all those requirements and looks most consistent of the possible solutions.

}
}
</style>
6 changes: 6 additions & 0 deletions src/components/NcAppNavigationSettingsButton/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

export { default } from './NcAppNavigationSettingsButton.vue'
1 change: 1 addition & 0 deletions src/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export { default as NcAppNavigationNew } from './NcAppNavigationNew/index.js'
export { default as NcAppNavigationNewItem } from './NcAppNavigationNewItem/index.js'
export { default as NcAppNavigationSearch } from './NcAppNavigationSearch/index.js'
export { default as NcAppNavigationSettings } from './NcAppNavigationSettings/index.js'
export { default as NcAppNavigationSettingsButton } from './NcAppNavigationSettingsButton/index.js'
export { default as NcAppNavigationSpacer } from './NcAppNavigationSpacer/index.js'
export { default as NcAppSettingsDialog } from './NcAppSettingsDialog/index.js'
export { default as NcAppSettingsSection } from './NcAppSettingsSection/index.js'
Expand Down
Loading