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(ui): Settings > Option to disable tab view #276

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

kobenguyent
Copy link
Collaborator

@kobenguyent kobenguyent commented Oct 9, 2024

resolves #60

  • provide an option to show/hide tabs
  • add refresh button to settings modal
  • add object-curly-spacing to eslint

Copy link

netlify bot commented Oct 9, 2024

Deploy Preview for chimerical-kitsune-a0bfa0 ready!

Name Link
🔨 Latest commit e403eab
🔍 Latest deploy log https://app.netlify.com/sites/chimerical-kitsune-a0bfa0/deploys/6707c5ab2b2b440008297fcb
😎 Deploy Preview https://deploy-preview-276--chimerical-kitsune-a0bfa0.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@flawiddsouza
Copy link
Owner

Rename this to showTabs:
image
disableShowTabs seems incorrect, because tabs are shown when it is true.


This declaration in methods is not required:
image
As you can directly use the top level functions in the vue object. You can only not access them in the template. It's also not being used.


Please rename the local storage key from LOCAL_STORAGE_KEY.SHOW_TAB_MODE to LOCAL_STORAGE_KEY.SHOW_TABS. The _mode implies there are different modes when there's only a false and true state.

Also here:
image
Just make everything show_tabs or SHOW_TABS. remove the mode.


This || string is not required in the type:
image
As you're controlling the method that returns AppConfig. Make it so it always returns true or false, which I think you're already doing.


There's an 1px extra border that's coming with this v-if:
image

You have to v-if where the TabBar component is being used. Then this won't happen:
v-if on div inside component:
image
Component v-if:
image

@flawiddsouza flawiddsouza merged commit 763a4b5 into main Oct 10, 2024
6 checks passed
@flawiddsouza flawiddsouza deleted the feat-option-to-hide-tab branch October 10, 2024 12:20
@flawiddsouza
Copy link
Owner

Hey @kobenguyent, just make a note of what I did in a41ee9c. Refresh page is not required as we have other settings that we dynamically update. This is how most settings are loaded in the app. From the store.

@kobenguyent
Copy link
Collaborator Author

Hey @kobenguyent, just make a note of what I did in a41ee9c. Refresh page is not required as we have other settings that we dynamically update. This is how most settings are loaded in the app. From the store.

Thanks for the update. That sounds better than refreshing page manually.

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.

Unclear on the purpose of the tabs
2 participants