Skip to content

Conversation

@HaNdTriX
Copy link
Contributor

@HaNdTriX HaNdTriX commented Oct 21, 2018

I have rewritten the with-antd-mobile example to make it simpler and easier to understand.

  • migrated to with-antd-mobile@2
  • dropped deprecated polyfills
  • used next-css
  • removed complex pages (this is now userland)
  • removed i18n (this is now userland)
  • added global font styles

@Enalmada can you give me a review on this?

Closes #5419

- Reduced noise
- Simplify setup
@HaNdTriX HaNdTriX force-pushed the examples/with-antd-mobile branch from 2100492 to 98eb0ff Compare October 21, 2018 16:51
@Enalmada
Copy link
Contributor

Enalmada commented Oct 21, 2018

It is good you have simplified the example. An example should only have the minimum necessary to empower the user to add anything else they want. I do feel it would be convenient to leave the MenuBar.js. It shows off a few things like routing onPress={() => Router.push(link)} that let new users hit the ground running. I would think it would be fine if the links each are going to a few sample pages equivalent to what you have now with index but different wording showing the routing is working.

I am concerned about the lack of any kind of icon example. Will new users have any problems using the default provided icons without the require-hacker and svg-sprite-loader example code in next.config.js? No one is ever gonna figure out how to get that stuff in the original next.config.js back if it is necessary to get icons working. I am really hoping none of that is actually necessary but worried that it is and users will hit a brick wall when trying to get icons working.

@HaNdTriX
Copy link
Contributor Author

@Enalmada Thanks for your feedback 🙏. I improved the example so some basic usecases are covered.

Afaik you don't need any special config like require hacker or some svg loader. Icons should just work 😃.

@timneutkens timneutkens merged commit dccbc1e into vercel:canary Nov 2, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants