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

feat(useFullscreen): support page fullscreen #1893

Merged
merged 18 commits into from Mar 23, 2023

Conversation

eveningwater
Copy link
Contributor

@eveningwater eveningwater commented Sep 28, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

暂无

💡 Background and solution

增加可以允许只设置浏览器全屏的配置。

📝 Changelog

Language Changelog
🇺🇸 English useFullscreen support full screen of page
🇨🇳 Chinese useFullscreen 支持页面全屏

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@brickspert
Copy link
Collaborator

这个能力不错,建议单独开一个 Hooks useBrowserFullscreen,每个 Hooks 代码简单一些,感觉会更好一些。

@brickspert
Copy link
Collaborator

然后 z-index 最好支持用户传入。

@eveningwater
Copy link
Contributor Author

eveningwater commented Sep 28, 2022

这个能力不错,建议单独开一个 Hooks useBrowserFullscreen,每个 Hooks 代码简单一些,感觉会更好一些。

这个也没加多少代码,集成到这个hooks里面也可以吧。

@eveningwater
Copy link
Contributor Author

然后 z-index 最好支持用户传入。

嗯这个可以加上

@eveningwater
Copy link
Contributor Author

然后 z-index 最好支持用户传入。

大佬,我想了一下,这个既然是全屏了,那应该就要呈现层级是最高的,所以这个z-index应该不需要自己配置吧,所以我个人的看法是将z-index设置的更大一点,比如9999,不知道我这么想可以不?

@eveningwater
Copy link
Contributor Author

这个能力不错,建议单独开一个 Hooks useBrowserFullscreen,每个 Hooks 代码简单一些,感觉会更好一些。

然后单独开一个hooks,也没有必要吧,因为加入的代码量也不算很多,也不影响维护吧,所以就融合进useFullscreen就可以了。

@liuyib liuyib self-requested a review March 9, 2023 14:32
@liuyib liuyib changed the title Update use fullscreen feat(useFullscreen): support browser fullscreen Mar 9, 2023
@liuyib liuyib added this to In progress in ahooks tasks via automation Mar 9, 2023
@liuyib liuyib changed the title feat(useFullscreen): support browser fullscreen feat(useFullscreen): support page fullscreen Mar 9, 2023
liuyib
liuyib previously approved these changes Mar 9, 2023
@liuyib liuyib self-requested a review March 9, 2023 17:01
@liuyib liuyib dismissed their stale review March 9, 2023 17:03

测试的还存在一些问题

ahooks tasks automation moved this from In progress to Review in progress Mar 9, 2023
@liuyib
Copy link
Collaborator

liuyib commented Mar 11, 2023

@crazylxr 我对原 PR 做了些优化,可以 review 了

@crazylxr crazylxr merged commit b3fd1bb into alibaba:master Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
ahooks tasks
Review in progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants