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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce useIsMobile and useIsFullscreen composables #4761

Merged

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Nov 6, 2023

Both composables store and return reactive state in module-scope (aka global) and are updated on resize.

Old mixins are now using new composables and are marked deprecated.

Fully compatible with Vue 3

Usage example in Options API:

<script>
import { useIsMobile } from '@nextcloud/vue/dist/Composables/useIsMobile.js'

export default {
  setup() {
    return {
      isMobile: useIsMobile(),
    }
  },

  watch: {
    isMobile() {
      // handle
    },
  },
}
</script>

<template>
  <div v-if="isMobile">I'm on mobile</div>
</template>

馃毀 Tasks

  • Add useIsMobile composable, that stores and returns isMobile state in module-scope (aka global)
  • Add composables to build
  • Refactor isMobile mixin to use new composable and mark deprecated
  • Migrate all components to use composable instead of the mixin
  • The same with isFullscreen

馃弫 Checklist

  • 鉀戯笍 Tests are included or are not applicable
  • 馃摌 Component documentation has been extended, updated or is not applicable

@ShGKme ShGKme added the 2. developing Work in progress label Nov 6, 2023
@ShGKme ShGKme added this to the 8.0.0 milestone Nov 6, 2023
@ShGKme ShGKme self-assigned this Nov 6, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 7, 2023

P.S. It was originally discussed in #4758

@susnux susnux modified the milestones: 8.0.0, 8.1.0 Nov 8, 2023
@raimund-schluessler
Copy link
Contributor

@ShGKme Awesome, thanks a lot. Works well!

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Finally got some time for testing: Works nicely 馃殌

@susnux
Copy link
Contributor

susnux commented Nov 9, 2023

BTW for the tests you need to adjust the import in tests/unit/components/NcAppNavigation/NcAppNavigation.spec.js

@raimund-schluessler raimund-schluessler force-pushed the feat/is-mobile--is-fullscreen--composables branch from f7d177d to 353bc49 Compare November 11, 2023 12:40
@raimund-schluessler
Copy link
Contributor

@ShGKme I rebased and adjusted the tests of NcAppNavigation. I hope, that's ok for you.

@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews feature: app-navigation Related to the app-navigation component feature: app-sidebar Related to the app-sidebar component feature: app-content Related to the app-content component and removed 2. developing Work in progress feature: app-sidebar Related to the app-sidebar component labels Nov 11, 2023
@raimund-schluessler
Copy link
Contributor

@ShGKme Can we go ahead here? I didn't want to merge your PR without you agreeing to it 馃槈

@ShGKme ShGKme marked this pull request as ready for review November 14, 2023 01:08
ShGKme and others added 4 commits November 14, 2023 06:08
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Raimund Schl眉脽ler <raimund.schluessler@mailbox.org>
@ShGKme ShGKme force-pushed the feat/is-mobile--is-fullscreen--composables branch from 353bc49 to 6237954 Compare November 14, 2023 01:08
@raimund-schluessler raimund-schluessler merged commit c92cb84 into master Nov 14, 2023
16 checks passed
@raimund-schluessler raimund-schluessler deleted the feat/is-mobile--is-fullscreen--composables branch November 14, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews feature: app-content Related to the app-content component feature: app-navigation Related to the app-navigation component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants