-
Notifications
You must be signed in to change notification settings - Fork 130
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
Theme: adding theme template #2390
base: master
Are you sure you want to change the base?
Conversation
eaa72f5
to
d5acf8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'adore la restructuration que tu as fais du markup de la page. Good job.
Il va falloir ajouter une page d'exemple soit pour le dernier menu item ou soit pour un des avants derniers item du menu. Ceci afin de bien voir l'effet de la class "active" qui est utilisé dans le menu de gauche pour identifier l'item courant.
d5acf8d
to
6dd44be
Compare
6dd44be
to
01f3e24
Compare
01f3e24
to
9519454
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Is that theme page should still have the "Top menu" button?
If not, then the "sign in" button for the "Jobs" theme example need to be at the same line of the breadcrumb. (I didn't tested that use case if that is already supported or not yet.)
@Garneauma can you add in the request description our internal reference number associated with this PR? thanks |
9519454
to
8506ffc
Compare
8506ffc
to
d019dbd
Compare
d019dbd
to
239e198
Compare
239e198
to
30b99f9
Compare
30b99f9
to
d9be7db
Compare
Dependent on PR #2410 being merged.
Changes related to: WET-467