Skip to content

[FE][SYNC-232] SYNC-V2 New Navbar and UI components - #68

Open
andyrochi wants to merge 48 commits into
upgrade-tiptapfrom
feature/SYNC-232
Open

[FE][SYNC-232] SYNC-V2 New Navbar and UI components#68
andyrochi wants to merge 48 commits into
upgrade-tiptapfrom
feature/SYNC-232

Conversation

@andyrochi

@andyrochi andyrochi commented Nov 14, 2022

Copy link
Copy Markdown
Contributor

Description:

  • New navbar component
  • New UI component SyncButton

Changes:

  • Implement new UI changes to navbar
  • Introduced a new SYNC UI component: <SyncButton> with similar interfaces as bootstrap-vue buttons
    • With props size, pill, variant (primary, tertiary), squared
  • Implement all UI elements in navbar without using bootstrap
  • Implement new profile dropdown
  • Implement new notification dropdown
  • Implement new search bar
  • Add query params in profile page to navigate to correct tab from profile dropdown

TODO:

  • Hamburger menu and RWD, since the UX team have not designed the UI yet
  • Notification dropdown and firebase integration
  • 熱門時事 (not designed)
  • 最新編輯 (not designed)

Test Scope:

Screenshots (optional)

  • Default (no user login)

image

  • User login

image

  • Explore dropdown

image

  • Notification dropdown

image

  • Profile dropdown

image

@yccodr yccodr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

image
The text in the dropdown should be aligned with the title.

Comment thread src/components/Post/BlockEditor.vue Outdated
Comment thread src/main.js
const req = require.context('@/assets/icons', true, /\.svg$/)
requireAll(req)
Vue.component('icon', SvgIcon)
Vue.component('SyncButton', SyncButton)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why SyncButton should be globally registered? Can't we import it when we need it?

@andyrochi andyrochi Dec 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer making it more like BootstrapVue, where components are globally visible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Any thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's fair enough, but I think importing the component when we need it would be better. It would shrink down the chunk size after the build.

Comment thread src/views/new/Post.vue Outdated
await loginFunc()
this.$router.back()
const redirect = this.getRedirectPath
if (redirect) { this.$router.push({ path: redirect.redirect }) } else { this.$router.back() }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the router starts at '/login', then there's no entry in the history stack. this.$router.back() won't work.
Maybe add more checks to prevent this behavior? (e.g. by checking router.START_LOCATION)

@david20571015
david20571015 removed their request for review May 7, 2024 16:42
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.

2 participants