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

Add dns record instruction page #545

Merged
merged 9 commits into from Apr 11, 2023
Merged

Conversation

Myrfion
Copy link
Contributor

@Myrfion Myrfion commented Apr 7, 2023

  • add dns record instruction page content
  • add component for faq section item

I think that this one has a good potential for expansion, such as adding more faq sections and more instructions

closes #443

@Myrfion Myrfion added category: front end Front end part of our web service area: web Web development related things [front end/back end] labels Apr 7, 2023
@Myrfion Myrfion added this to the Milestone 0.9 milestone Apr 7, 2023
@Myrfion Myrfion self-assigned this Apr 7, 2023
Copy link
Contributor

@Ririio Ririio left a comment

Choose a reason for hiding this comment

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

image

I was wondering couldn't you use Tabs here to make the page look more organized?

@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 7, 2023

image

I was wondering couldn't you use Tabs here to make the page look more organized?

@Ririio love this idea, did you implement it in your environment on my branch? If so feel free to push

@Ririio
Copy link
Contributor

Ririio commented Apr 8, 2023

I was wondering couldn't you use Tabs here to make the page look more organized?

@Ririio love this idea, did you implement it in your environment on my branch? If so feel free to push

Oh I don't think it's a good idea for me to mess around with your branch, it's simple to implement actually, just follow the example here

app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
@humphd
Copy link
Contributor

humphd commented Apr 8, 2023

I like @Ririio's UI with tabs, that's a good idea. It mimics what we're doing in certificates too.

@Myrfion Myrfion requested review from humphd and Ririio April 8, 2023 17:13
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Nice improvements. A few small things.

app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
@Myrfion Myrfion requested a review from humphd April 8, 2023 21:13
humphd
humphd previously approved these changes Apr 8, 2023
@Eakam1007
Copy link
Contributor

Eakam1007 commented Apr 8, 2023

I think it would be nice to make the question text pop out more compared to the answer text:

image

Maybe increase the font size or something else

@humphd
Copy link
Contributor

humphd commented Apr 9, 2023

Looks like this needs a rebase too to get the changes to the main DNS Records page.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

There some weird alignment issues with your numbered list:

Screenshot 2023-04-10 at 10 32 06 AM

@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 10, 2023

There some weird alignment issues with your numbered list:

Screenshot 2023-04-10 at 10 32 06 AM

resolved this one

@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 10, 2023

image

@humphd humphd linked an issue Apr 10, 2023 that may be closed by this pull request
@humphd humphd removed the request for review from Eakam1007 April 10, 2023 15:45
app/components/instructions/faq-accordion.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/instructions.tsx Outdated Show resolved Hide resolved
@Ririio
Copy link
Contributor

Ririio commented Apr 10, 2023

image

I would add a much bigger space between each list number, because stacking them all close to each other makes it harder to read

@Myrfion Myrfion requested review from humphd and Genne23v April 10, 2023 23:52
Genne23v
Genne23v previously approved these changes Apr 10, 2023
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

R+ from me with the eslint fix.

SerpentBytes
SerpentBytes previously approved these changes Apr 11, 2023
humphd
humphd previously approved these changes Apr 11, 2023
@Myrfion Myrfion dismissed stale reviews from humphd, SerpentBytes, and Genne23v via d667e04 April 11, 2023 11:49
Copy link
Contributor

@Ririio Ririio left a comment

Choose a reason for hiding this comment

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

I'm not sure if it's necessary, but for FAQ, I think user might ask what the difference is between a "dns-record" and "domain"

@Myrfion Myrfion merged commit df8486b into main Apr 11, 2023
8 checks passed
@humphd
Copy link
Contributor

humphd commented Apr 11, 2023

@Myrfion please rebase and squash this properly, don't merge main.

@Myrfion Myrfion deleted the feature/dns-records-instructions branch April 11, 2023 14:24
@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 11, 2023

@Myrfion please rebase and squash this properly, don't merge main.

Sorry I merged in main, before your message 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: web Web development related things [front end/back end] category: front end Front end part of our web service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dedicated Page for DNS-Records Information
6 participants