Skip to content

feat: add option to display framework icon #21

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

Merged
merged 13 commits into from
Jun 19, 2023
Merged

feat: add option to display framework icon #21

merged 13 commits into from
Jun 19, 2023

Conversation

undead-voron
Copy link
Contributor

No description provided.

@atinux
Copy link
Member

atinux commented May 22, 2023

Could you share a video about the result please?

@undead-voron
Copy link
Contributor Author

icon_change.mp4

@atinux

Checked following stuff:

  • brand icons are disabled by default
  • checked brand icons on different frameworks
  • checked that brand icons will return to default for opened tabs after feature is disabled
  • checked brand icons when multiple tabs are active
  • checked that tab state is saved after service worker has been deactivated

@undead-voron
Copy link
Contributor Author

@atinux
I think I can't edit reviewers for this pull request. Can you add yourself as reviewer or should I close this pr and create a new one?

@atinux atinux changed the title Adaptive icon feat: add option to display framework icon Jun 8, 2023
@atinux
Copy link
Member

atinux commented Jun 8, 2023

Nice work @undead-voron and sorry for late answer I was on vacation.

I think we can display the icons by default and remove the ability to choose, this way we don't need to add the storage permission. Happy to update your PR?

@undead-voron
Copy link
Contributor Author

Sure @atinux . Please, update it if you got time.

@undead-voron
Copy link
Contributor Author

@atinux
I think that we need to have storage permission anyway. As far as chrome extension has service worker in manifest version 3 (contrary to background page in manifest version 2 ) we need to store tabs state in storage (otherwise there would be no info in popup after service worker terminates).

Copy link
Member

atinux commented Jun 8, 2023

I see, don't have much time right now.

I think we can keep storage but removing the settings to display the icons by default, I prefer this new behavior :)

@undead-voron
Copy link
Contributor Author

Sounds great!

@undead-voron
Copy link
Contributor Author

undead-voron commented Jun 11, 2023

Hi @atinux
I had some free tine to removed settings UI for choosing framework icon and make framework icon to be displayed by default.
here are the commits
567f408
e036bcc
4cc3cdb

please, review current pr one more time, when you got time for it

@atinux atinux merged commit 1526345 into nuxtlabs:master Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants