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

93 enhance page header social share panel #103

Merged
merged 13 commits into from
Nov 23, 2023

Conversation

Khalil-NOUI
Copy link
Contributor

@Khalil-NOUI Khalil-NOUI commented Nov 21, 2023

-enable tab title meta Info and enable share feature in the single event page
-enhance translation in some lacking components

@Khalil-NOUI Khalil-NOUI added the enhancement New feature or request label Nov 21, 2023
@Khalil-NOUI Khalil-NOUI added this to the Performance Optimization milestone Nov 21, 2023
@Khalil-NOUI Khalil-NOUI self-assigned this Nov 21, 2023
@Khalil-NOUI Khalil-NOUI linked an issue Nov 21, 2023 that may be closed by this pull request
Copy link

vercel bot commented Nov 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
aidebeaide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 4:28am
togatherwebuild ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 4:28am

Copy link
Collaborator

@HachemBouhadede HachemBouhadede left a comment

Choose a reason for hiding this comment

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

let's not forget to delete all unceccessary comment and console.logs ! hope this gets solved soon

{error}
</p>
<>
<IndexPage title='Edit Profile' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be in the profile/[id]/editProfile ? it shouldn't be in componenets

<HiChevronRight />
</div>
<>
<IndexPage title={`${eventDisplay.title}`} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

same headr, i supposed headers go to pages not componenets

Copy link
Collaborator

Choose a reason for hiding this comment

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

but i understand why you're using it her, anyway, as long as it works <3

{t("editEvent.intro")}
</p>
<>
<IndexPage title={`Edit Event ${oldInfo.title}`} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Collaborator

Choose a reason for hiding this comment

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

i get the logic of writing as a component, i think it's actually brilliant, but with all these changes, i think the standard way could've easier, anyway, hope this gets solved out soon ! and great efforts khalil

<div className='w-7 h-7 bg-gray-50 rounded-full text-red-500 flex justify-center items-center'>
<LuFilterX />
<>
<IndexPage title='All Events' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aide for Aide Events or Our Events ! could be nice :D

{t("createEvent.intro")}
</p>
<>
<IndexPage title='Create an Event' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

i kinda unerstand why you'd use the indexPage components in componenets that are meant for dynamic pages, but for static pages, i defently think it's prefarble to put them in pages

/>
)}
<>
<IndexPage title={`${userData.displayName} events`} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah this seems more logical

@@ -15,6 +16,7 @@ export default function HomePage({ props }) {

return (
<>
<IndexPage title='Pebble' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't forget to update the title

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aide be Aide :)

@HachemBouhadede
Copy link
Collaborator

image

current error,
i did a quick search and found that it's okey to use a header inside nested componenets instead of a page; so that theoretically isn't an issue, but you never know, if you get no luck try to reach to a trainer

@Khalil-NOUI
Copy link
Contributor Author

@HachemBouhadede
Thank you,Hacheim for the feedback, I'll address of it as much as i can in the next push

@HachemBouhadede
Copy link
Collaborator

HachemBouhadede commented Nov 22, 2023

still same issue with vercel
image

here's a suggution idea @Khalil-NOUI for this page make the header a normal one don't use the indexPage componenet just to solve it out

…ge, improve tranlsation

BREAKING CHANGE: head translation

resolves #93
@Khalil-NOUI
Copy link
Contributor Author

All good to GO !,

@HachemBouhadede HachemBouhadede merged commit e017028 into develop Nov 23, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enhance page header & social share panel
2 participants